Validation on configuration using interfaces doesn't work

[edeandrea]

Describe the bug
According to https://quarkus.io/guides/config-reference#additional-notes-on-configproperties I should be able to validate nested configuration using interfaces but it doesn't seem to work.

Expected behavior
I would expect the application to fail when starting up

Actual behavior
The application starts up without error

To Reproduce
Reproducer application attached

quarkus-config-sample.zip

[edeandrea]

Just to add an additional comment - when using interfaces, putting validation on the getters doesn't work in general.

@ConfigProperties(prefix = "greeting")
public interface GreetingConfiguration {
	String getMessage();

	@ConfigProperty(defaultValue = "!")
	String getSuffix();

	Optional<String> getName();

	@Positive
	Integer getPrizeAmount();
}

with configuration

greeting.message=hello
greeting.name=quarkus
greeting.prize-amount=0

should produce an error at startup, but it does not

[geoand]

I'll take a look this week

[geoand]

Your code isn't correct:

You need to add @Valid to content like so:

	@Valid
	private ContentConfig content;

That results in the proper error message being triggered.

As for the issue with validation not being triggered on interfaces, I can confirm it

[edeandrea]

THanks @geoand I can confirm adding @Valid to the getter works. Thanks very much for that!

Do you want me to open a separate issue for the interfaces? Or just continue with this issue for that (I can update the reproducer example).

[geoand]

I'd say just update the title, description and reproducer for this one is fine

[edeandrea]

Done.

I also noticed that in the IDE (IntelliJ in this specific case - not sure about behavior in others) that the application.properties file does not recognize the nested properties (see screenshot)

[geoand]

Done.

I also noticed that in the IDE (IntelliJ in this specific case - not sure about behavior in others) that the application.properties file does not recognize the nested properties (see screenshot)

Not much I can do about that...

[edeandrea]

Is it expected that validation should work when using interfaces?

[geoand]

Not really. I'm looking into how it can be added.

[geoand]

This can be done, but not without adding a ton of complexity to the @ConfigProperties implementation.

So unless this gets asked for again, I'll hold off on implementing it.

[edeandrea]

Noted - but I might add something to the documentation mentioning that validation isn't supported/implemented when using interfaces.

Honestly I only discovered it when looking at the docs and thinking it was cool. Reduces a lot of boilerplate.

[geoand]

Noted - but I might add something to the documentation mentioning that validation isn't supported/implemented when using interfaces.

That's a good point

[geoand]

#15086 is what is actually necessary to make this happen

[edeandrea]

Looks good to me! Ship it! :D

[geoand]

I'll need to rethink it, not to mention add the necessary tests (it did work on your example though)

[radcortez]

@geoand we can probably think about dropping @ConfigProperties interface implementation and replace it with SR Config @ConfigMapping, even if we just substitute the annotation at build time.

Right now, it seems we are just duplicating effort. Regarding validation, @ConfigMapping does not support that yet, but I already have a prototype for it here: smallrye/smallrye-config#488

We can check some other things that we are missing and port them to SR. We also support other stuff that @ConfigProperties do not support, so at some point we should decide what to do.

[edeandrea]

Whatever the decision going forward is let's just keep in mind the top-level use case:

As a developer I should be able to expose type-safe configuration and/or hierarchies of configuration and be able to validate the configuration against a set of rules.

Currently the "interface-way" of defining configuration doesn't support that requirement whereas the "class-way" does. If the "interface-way" continues to be a "thing", then it should support the requirement in some fashion, no?

[geoand]

@radcortez yes, let's sync sometime before 1.13 and name a decision

[radcortez]

Currently the "interface-way" of defining configuration doesn't support that requirement whereas the "class-way" does. If the "interface-way" continues to be a "thing", then it should support the requirement in some fashion, no?

No worries. This is just the internal implementation. It shouldn't matter to the end user. And yes, interfaces should support validation as well.