Quarkus 2.1.1 → 2.1.2 drops support for configuration classes

[kny78]

Describe the bug

We use classes with @ConfigMapping and it worked fine until 2.1.1, but in 2.1.2 it requires us to use interfaces.
This implies large changes to our code, and we loose the ability to add metods to our configuration objects.

I have made a test-project that illustrates the problem:
See: https://github.com/kny78/quarkus-config-class-vs-interface

See output from running with 2.1.1 and 2.1.2 with Github actions:
error triggered by github actions

Expected behavior

No exception and configuration works as it should.

Actual behavior

Exception thrown:

[ERROR] com.example.GreetingResourceTest.testHelloEndpoint Time elapsed: 0.013 s <<< ERROR! java.lang.RuntimeException: java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors [error]: Build step io.quarkus.arc.deployment.ConfigBuildStep#generateConfigClasses threw an exception: java.lang.IllegalStateException: SRCFG00043: The @ConfigMapping annotation can only be placed in interfaces, class com.example.MyConfig is a class at io.smallrye.config.ConfigMappingLoader.validateAnnotations(ConfigMappingLoader.java:95) at io.smallrye.config.ConfigMappingLoader.getConfigMappingClass(ConfigMappingLoader.java:44) at io.smallrye.config.ConfigMappingLoader.getConfigMappingInterface(ConfigMappingLoader.java:40) at io.smallrye.config.ConfigMappingLoader.getConfigMappingsMetadata(ConfigMappingLoader.java:34) at io.quarkus.arc.deployment.ConfigBuildStep.generateConfigClasses(ConfigBuildStep.java:289) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at io.quarkus.deployment.ExtensionLoader$2.execute(ExtensionLoader.java:820) at io.quarkus.builder.BuildContext.run(BuildContext.java:277) at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18) at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2449) at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1478) at java.base/java.lang.Thread.run(Thread.java:829) at org.jboss.threads.JBossThread.run(JBossThread.java:501)

How to Reproduce?

Just run the project as described in the README in the example project

Output of uname -a or ver

Linux fedora 5.13.8-200.fc34.x86_64 #1 SMP Wed Aug 4 19:59:54 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "11.0.12" 2021-07-20

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

Maven home: /home/kny/.m2/wrapper/dists/apache-maven-3.8.1-bin/2l5mhf2pq2clrde7f7qp1rdt5m/apache-maven-3.8.1

Additional information

Code is generated from https://code.quarkus.io. Only small changes to the application.properties code and java code.

[gsmet]

/cc @radcortez

[radcortez]

Hi @kny78

Unfortunately, @ConfigMapping was never meant to work with classes. Classes have some limitations that we experienced with the old @ConfigProperties and that is why we deprecated them. Also, unfortunately MP Config, also added support to class based configuration and we added some limited support to comply with the specification (BTW, MP Config is currently discussing dropping the class based configuration, due to the same limitations we already knew from the previous experience).

The @ConfigMapping supporting classes was just a side effect of the support we added to org.eclipse.microprofile.config.inject.ConfigProperties. Not all the features are implemented for classes, which causes confusion to the user, check for instance #18672, which was the main motivator to add the check and disallow @ConfigMapping completely in classes.

To keep using classes, you will need to use org.eclipse.microprofile.config.inject.ConfigProperties instead, with the recommendation to move to interfaces. This may be removed from MP Config in a future version as well.

[kny78]

Hi @radcortez

I think we are discussing two issues here:

1. Expected changes in patch releases. (E.g. 2.1.1 to 2.1.2)
2. How to config system works in Quarkus

1. Expected changes in patch releases

I would expect the semver rules to apply. Semver defines the following for x.y.z upgrades:

• Major upgrades (x): Big upgrades, including breaking changes. No guarantee on compatibility.
• Minor upgraded (y): New features, but no breaking changes. (Only very small breaking changes)
• Patch (z): No breaking changes, and no new features. Only bugfixes, that should not break any compatibility.

2. How to config system works in Quarkus

I am working in Kotlin, and would prefer to have "data classes", with the values, and simple conventions to [Long, Double, String, Array, Map, Data/Time mapping, Duration].

But I see that Smallrye / Quarkus wants to use interfaces in stead, and I don't know enough about it to disagree :-)

But can you add a @SmallryeIgnore annotation or similar, that we can mark methods that is not configuration methods, such that we can have methods on the interface that is not directly bound to the configuration?

Regards,
Kjetil

[radcortez]

1. Expected changes in patch releases

I would expect the semver rules to apply. Semver defines the following for x.y.z upgrades:

I understand the frustration, but this was never meant to work. I was quite surprised that it actually worked. As I mentioned, the only reason why it worked is that we added some last minutes changes to support the MP Config case, which uses a completely different annotation. If you look in the original PR for the feature, there was never any class support: smallrye/smallrye-config#333. Also, the documentation states that you require an interface and not a class: https://quarkus.io/guides/config-mappings. In my mind, we are not breaking any semver rules, because it was never a feature, to begin with, but of course, I respect your opinion :)

2. How to config system works in Quarkus

I am working in Kotlin, and would prefer to have "data classes", with the values, and simple conventions to [Long, Double, String, Array, Map, Data/Time mapping, Duration].

Some of the big issues with classes are that they require heavy use of reflection to modify the fields (if they are not public), or to call setters on them. Fields may have been already initialized (what should we do if a collection already has elements added by the constructors? are these considered defaults? should we override them?), the class may have mutable state (config instances are global and should be immutable, so having a way to change them from anywhere is dangerous). There are some more details but in the end we thought it was too much work for little gain, creates too many edge cases, compared to a clean approch using an interface. But hey, we are always open for PR, so if someone was to implement the full support on that, we are more than happy to review it and add it :)

But can you add a @SmallryeIgnore annotation or similar, that we can mark methods that is not configuration methods, such that we can have methods on the interface that is not directly bound to the configuration?

Yes, we can certainly do that.

[kny78]

I guess that leaves the conclusion, that if we get @SmallryeIgnore or @ConfigTransient to exclude functions that should not be part of the mapping, it is not too hard for us to change to the new approach :-)

[radcortez]

Ok, let me see what I can do. Can you also provide me an example of your kt configuration? Thanks!