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

API for using mapped configurations in a standalone way #1001

Open
dmlloyd opened this issue Sep 21, 2023 · 10 comments · May be fixed by #1021
Open

API for using mapped configurations in a standalone way #1001

dmlloyd opened this issue Sep 21, 2023 · 10 comments · May be fixed by #1021
Labels
enhancement New feature or request

Comments

@dmlloyd
Copy link
Contributor

dmlloyd commented Sep 21, 2023

Mapped configurations are very good in terms of brevity, clean API, and usability. However, they are not usable unless you're using SmallRye Config to read a configuration and create them indirectly in that way.

As a configuration user, I would like to have the ability to use mapped configurations to configure my application using my application's API, but have the usage of the actual configuration file be optional. This way I can configure my application using a file, or programmatically, all using the same API.

To allow this, we should have an API which allows configuration interfaces to be instantiated directly using a builder API. The API could look something like this:

@ConfigMapping(prefix = "hello-world")
public interface HelloWorldConfig {
    @WithDefault("hello")
    String helloMessage();

    @WithDefault("5")
    int numberOfTimes();

    Optional<Path> outputPath();
}
    public static void main(String[] args) {
        ConfigMappingBuilderFactory factory = ...;
        // manually create a config
        ConfigMappingBuilder<HelloWorldConfig> configBuilder = factory.builder(HelloWorldConfig.class);
        configBuilder.set(HelloWorldConfig::helloMessage, "good morning!");
        // convenience for optional properties
        configBuilder.setOptional(HelloWorldConfig::outputPath, Path.of("/tmp/good-morning.txt"));
        // primitives without boxing
        configBuilder.set(HelloWorldConfig::numberOfTimes, 10);
        // oops, I made a mistake, let's revert to default
        configBuilder.unset(HelloWorldConfig::numberOfTimes);
        HelloWorldConfig config = configBuilder.build();
        // now do something with config
    }

An API with this style is easier to use than source code generation based approaches. The builder factory would track generated classes for each configuration interface, which ideally could be reused by the existing config mapping implementation (though that implementation would not use the builder API itself per se). When running on Java 16 and later, these implementation classes should likely be implemented as records.

The generated record would be able to enforce constraints like non-nullity and validation in its canonical constructor (ideally using smallrye-common-constraint to validate nullity and smallrye-config-validator for the user validation rules as expected), ensuring that the consuming code will always receive a correct object. Optional properties would default to Optional.empty() and properties with explicit defaults would have those defaults set if no value is given.

Note that by waiting for the baseline JDK to move from 11 to 17, in addition to using records on the back end, we could possibly use my backport of the JDK classfile API to make class generation easiest.

@dmlloyd
Copy link
Contributor Author

dmlloyd commented Sep 21, 2023

This relates a bit to #981 because a generated implementation class could fairly easily "know how" to parse itself out of a configuration subset without involvement of the kinds of things that we currently employ internally for that purpose (like name iterators). This would in turn simplify the mapper a bit more.

@radcortez
Copy link
Member

Sounds good.

I still have a bunch of things that I want to do first, but definitely something worth supporting.

@dmlloyd
Copy link
Contributor Author

dmlloyd commented Oct 2, 2023

It is not practical to transition some Quarkus extensions (like logging, various HTTP extensions) to config mapper without this feature. They rely on manually instantiating and populating configuration objects when the server fails to start in dev mode by way of the somewhat hacky io.quarkus.runtime.configuration.ConfigInstantiator, which could potentially get replaced in a cleaner way using this feature.

@radcortez
Copy link
Member

For such cases, we use the following workaround:

https://github.com/quarkusio/quarkus/blob/5f90f04945cef466a2230bf0d7208412ab5e54d4/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java#L248-L251

Not ideal, but it works better than the ConfigIntantiator.

@dmlloyd
Copy link
Contributor Author

dmlloyd commented Oct 3, 2023

Since I'm at a point where I could use this API in at least 3 different places, would you be OK if I contributed this piece? Or, do you already have a start on it?

@dmlloyd
Copy link
Contributor Author

dmlloyd commented Oct 3, 2023

For such cases, we use the following workaround:

https://github.com/quarkusio/quarkus/blob/5f90f04945cef466a2230bf0d7208412ab5e54d4/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java#L248-L251

Not ideal, but it works better than the ConfigIntantiator.

I guess this is more appropriate for #1002 but I wonder if we should consider allowing the configuration to be created even if it contains errors - for example, produce a tuple of the configuration models and any errors/warnings produced when reading the configuration?

@radcortez
Copy link
Member

Since I'm at a point where I could use this API in at least 3 different places, would you be OK if I contributed this piece? Or, do you already have a start on it?

Sure.

I actually started to look into it a couple of hours ago, but I've found some things we need to change first.

In Quarkus, we already bytecode generate during build time the mapping implementation. Unfortunately, we still need the introspection during runtime to generate the mapping actions. So we need to generate the actions during build time and bytecode generate the actions to be used at runtime. We also need APIs to create the BiConsumers via a builder or something else.

Unless you have another idea on how to build the generated implementations, we still require the BiConsumer actions for certain properties that we create in ConfigMappingProvider. We need to eliminate references and API calls to either ConfigMappingInterface or Property. In reality, we don't need these references for during runtime.

How about if I work on this piece and you can work on the mapping factory API?

dmlloyd added a commit to dmlloyd/smallrye-config that referenced this issue Oct 3, 2023
@dmlloyd dmlloyd linked a pull request Oct 3, 2023 that will close this issue
dmlloyd added a commit to dmlloyd/smallrye-config that referenced this issue Oct 3, 2023
dmlloyd added a commit to dmlloyd/smallrye-config that referenced this issue Oct 4, 2023
@dmlloyd
Copy link
Contributor Author

dmlloyd commented Nov 3, 2023

Status update/"What's taking so long?":

I'm looking at a couple of (solvable, but not yet solved) problems. One issue is handling of default values. When we read a configuration model from a real configuration, we use a last-resort configuration source to provide the values, so that all APIs will show the same default value behavior. However there is no configuration source involved when dealing with interfaces, so this behavior must be reimplemented directly using the converter and default value string of the property.

The second problem is that of validation. Our current validators operate as converters, meaning that their input is a string. The input for building a configuration object is not a string but an already-converted type. Therefore the converter-based approach is fundamentally incompatible.

I think that since validation is not fully implemented yet for config interfaces (other than the bean validation approach which is not preferred), we can still rethink it a little. I would propose a separate Validator<T> interface that could be used to reimplement our standard validations (the range ones at least). Then separately we can have a concept of input validations using regex and the Converter infrastructure if that is desired.

@dmlloyd dmlloyd closed this as completed Nov 3, 2023
@radcortez
Copy link
Member

I think that since validation is not fully implemented yet for config interfaces (other than the bean validation approach which is not preferred), we can still rethink it a little. I would propose a separate Validator<T> interface that could be used to reimplement our standard validations (the range ones at least). Then separately we can have a concept of input validations using regex and the Converter infrastructure if that is desired.

Sounds good.

@dmlloyd dmlloyd reopened this Nov 3, 2023
@dmlloyd
Copy link
Contributor Author

dmlloyd commented Nov 3, 2023

Closed on accident. They shouldn't make that button so attractive :)

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

Successfully merging a pull request may close this issue.

2 participants