@ConfigProperty with defaultValue="" are not working on master

[ia3andy]

Describe the bug
When using @ConfigProperty with a defaultValue="", it is never set with the defaultValue.

Expected behavior
"" should be used when no configuration is set

Actual behavior
the attribute is null

To Reproduce

1. Add a @ConfigProperty with defaultValue
2. Add a test which checks that it's not null

Environment (please complete the following information):

• Quarkus version or git rev: master

WE SHOULD HAVE SOME TEST FOR IT BEFORE THE NEXT RELEASE

[dmlloyd]

Also relevant: eclipse/microprofile-config#475

Arguably, we should disallow null values for @ConfigProperty and require use of Optional instead to be consistent with the basic Config API. Related to eclipse/microprofile-config#446 as well.

[ia3andy]

test-default-value.zip

On this project, the test passes on 1.0.1.Final and fails on master.

The bug is narrowed down to only defaultValue="" not working, other case work

[dmlloyd]

This was discussed in chat. In conclusion, having a default value of "" effectively means "if this value is not given, it is empty", which is already the effective default value for all properties.

Empty values (as discussed in eclipse/microprofile-config#446) are used when a value is missing. So in order to represent a value that can be emptiable, be it a String, a collection type, etc., one must declare it as Optional.

Because we haven't quite hammered out the spec inconsistencies in our implementation, injected values can sometimes result in null. But this is something that will be reported as an error ("A required property is missing") in the future.

So in summary: If a property can be empty, make it Optional. If there is a non-empty default value, use defaultValue. Both can be used in cases where it is appropriate for a property where the value is permitted to be empty but normally isn't.

The action items would be:

• Make sure this is covered in a migration guide somewhere
• Add the enforcement for required properties in the upstream SmallRye Config project
• Add Quarkus tests for the corrected behavior

[ia3andy]

as discussed in the chat, there is a bug with Optional not being empty unless defaultValue="" is used, this needs to be covered in that issue (or in another new one) too

[gsmet]

@ia3andy can you clarify the bug of the last comment? If you we get back to it in a week, we will have forgotten what it is about.

[ia3andy]

@gsmet here is a better description of what I meant:

An exception is thrown when using this without providing my-prop:

@ConfigProperty(name = "my-prop")
Optional<String> myProp

It should not fail and instead give a Optional.empty (as described by @dmlloyd above)

Note:
Adding defaultValue="" makes it work and return Optional.empty

@ConfigProperty(name = "my-prop", defaultValue="")
Optional<String> myProp

[dmlloyd]

This is fixed in upstream with smallrye/smallrye-config#209.

[gastaldi]

It looks like this is fixed with the Smallrye Config update. Reopen if the problem persists