Quarkus: @ConfigProperty of List<String> with empty default value fail with error

Created on 31 Mar 2020  路  15Comments  路  Source: quarkusio/quarkus

Describe the bug

Using this:

    @ConfigProperty(name = "items", defaultValue = "")
    List<String> getItems();

Expected behavior
Should return an empty list (at least I would expect)

Actual behavior
Throw an exception:

java.util.NoSuchElementException: Property config.items not found

To Reproduce

Run the tests in:
https://github.com/ia3andy/check-list-config-empty

Environment (please complete the following information):

  • Quarkus version or git rev: 1.3.1.Final
kinbug triaginvalid

All 15 comments

@geoand I know you love me right now :-)

As explained in: https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/Empty.20list.2Farray.20as.20the.20default.20value.20for.20a.20.40ConfigItem
the proper way to do what you want is to use:

   @ConfigProperty(name = "items")
    Optional<List<String>> getItems();

@geoand I still believe that's is an issue, IMO having a Optional<List<String>> is counter intuitive as Collection are already capable of being empty, why would we want to wrap it in an optional?

When you do things from intuition and it fails, I think we should, most of the time, consider it a bug..

I can see your point, no doubt :)

@dmlloyd up to you to reopen it ;-)

The provided solution is not working either: https://github.com/quarkusio/quarkus/issues/8294

I can confirm that Optional<List<String>> works for @ConfigItem (quarkus-specific extension config). However, the behavior is probably undefined in the spec and so it might not work for @ConfigProperty...

There is a bug in @ConfigProperties and it does not behave the same as @ConfigProperty does - I'm working on a fix now.

@geoand I still believe that's is an issue, IMO having a Optional<List<String>> is counter intuitive as Collection are already capable of being empty, why would we want to wrap it in an optional?

Because every config property may or may not be empty, yet not every type has an empty representation. Additionally, this is a natural consequence of the necessary logic to handle empty values consistently.

When you do things from intuition and it fails, I think we should, most of the time, consider it a bug..

I'm sorry you feel this way, but do not make the mistake of assuming that intuition is uniform to all, or that it is necessarily correct. After all, intuition tells us that the Earth is flat...

I'm sorry you feel this way, but do not make the mistake of assuming that intuition is uniform to all, or that it is necessarily correct. After all, intuition tells us that the Earth is flat...

LMAO :)

Because every config property may or may not be empty, yet not every type has an empty representation. Additionally, this is a natural consequence of the necessary logic to handle empty values consistently.

@dmlloyd I would handle all Optional<..> as we currently do, but I would also make it directly work with types which have an empty representation. This would also be a consistent way to handle empty IMHO.

I'm sorry you feel this way, but do not make the mistake of assuming that intuition is uniform to all, or that it is necessarily correct. After all, intuition tells us that the Earth is flat...

Well, do you want me to create a rocket trying to prove that Optional<List<..>> is an heresy 馃槀

... Maybe just a warning logs could at least direct users following their intuition to the right solution :-) (@geoand I am starting a book on warning logs 馃槀 馃槆)

Because every config property may or may not be empty, yet not every type has an empty representation. Additionally, this is a natural consequence of the necessary logic to handle empty values consistently.

@dmlloyd I would handle all Optional<..> as we currently do, but I would also make it directly work with types which have an empty representation. This would also be a consistent way to handle empty IMHO.

To do so would require extra handling specific to collections and strings. It doesn't seem worth it to me when we can use Optional and have it mean the exact same thing for every possible type. OTOH, a user can always use orElse to get whatever their desired empty value representation is, regardless of type.

I'm sorry you feel this way, but do not make the mistake of assuming that intuition is uniform to all, or that it is necessarily correct. After all, intuition tells us that the Earth is flat...

Well, do you want me to create a rocket trying to prove that Optional<List<..>> is an heresy

... Maybe just a warning logs could at least direct users following their intuition to the right solution :-) (@geoand I am starting a book on warning logs )

Put another way: what does "required" mean in this context? Our interpretation is that a required property cannot be empty, whereas an optional property may be empty. So if a List is required, it may not be empty, and if it is optional, it may be empty. So how do we represent this in code? Surely making optional things (including lists) be wrapped in Optional is the simplest and most obvious solution, whereas things that are not Optional are not optional.

Put another way: what does "required" mean in this context? Our interpretation is that a required property cannot be empty, whereas an optional property may be empty. So if a List is required, it may not be empty, and if it is optional, it may be empty. So how do we represent this in code? Surely making optional things (including lists) be wrapped in Optional is the simplest and most obvious solution, whereas things that are not Optional are not optional.

Ok ok, I think you convinced me :-)

When I have time I will update the configuration doc, your explanation here is really good. The doc should say that:

  • having defaultValue="" is never making sense, I think we should have a warning for that case (even for String right?).
  • required fields (non Optional) must either provide a non "" defaultValue or be defined.
  • Using Optional<...> with a defaultValue is not really making sense either.

Put another way: what does "required" mean in this context? Our interpretation is that a required property cannot be empty, whereas an optional property may be empty. So if a List is required, it may not be empty, and if it is optional, it may be empty. So how do we represent this in code? Surely making optional things (including lists) be wrapped in Optional is the simplest and most obvious solution, whereas things that are not Optional are not optional.

Ok ok, I think you convinced me :-)

When I have time I will update the configuration doc, your explanation here is really good. The doc should say that:

  • having defaultValue="" is never making sense, I think we should have a warning for that case (even for String right?).

Yeah effectively "" is always the default if no other is given.

  • required fields (non Optional) must either provide a non "" defaultValue or be defined.
  • Using Optional<...> with a defaultValue is not really making sense either.

Disagree on both of these. Whether a property has a default value is orthogonal to whether the property is required. For example, I could have an optional property of type List which contains "foo,bar" by default, but is also allowed to be empty (and the user could explicitly make it empty by giving an empty string). An example of this would be specifying default providers for some service, where the user wants no providers.

Thanks for all the info @dmlloyd, following our discussion, I created a housekeeping issue and will improve the doc asap: https://github.com/quarkusio/quarkus/issues/8306

Was this page helpful?
0 / 5 - 0 ratings