Inconsistent behavior for default configuration value between Set and Map

[ctron]

Describe the bug

Using Quarkus configuration, specifically @ConfigMapping:

• A Set (most likely a List too) cannot be empty, but must be wrapped inside an Optional if one wants a "default empty" behavior
• A Map cannot be wrapped in an Optional, but is empty by default

That is confusing.

Expected behavior

A less confusing approach.

I would argue that Lists, Maps, and Optionals (which can all be streamed) are empty by default.

However, I would absolutely expect that if I can/must have Optional<Set<>>, then I can also have Optional<Map<>>.

Actual behavior

It is confusing.

How to Reproduce?

Use try to use:

@ConfigMapping(prefix = "foo.bar")
public interface Configuration {

    Optional<Set<String>> foo();
    Optional<Map<String, String>> bar();
}

Output of uname -a or ver

Linux brocken 5.19.9-200.fc36.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Sep 15 09:49:52 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "17.0.4.1" 2022-08-12 OpenJDK Runtime Environment (Red_Hat-17.0.4.1.1-1.fc36) (build 17.0.4.1+1) OpenJDK 64-Bit Server VM (Red_Hat-17.0.4.1.1-1.fc36) (build 17.0.4.1+1, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.12.3.Final

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

Apache Maven 3.8.3 (ff8e977a158738155dc465c6a97ffaf31982d739) Maven home: /home/jreimann/tools/maven/apache-maven-current Java version: 17.0.4.1, vendor: Red Hat, Inc., runtime: /usr/lib/jvm/java-17-openjdk-17.0.4.1.1-1.fc36.x86_64 Default locale: en_IE, platform encoding: UTF-8 OS name: "linux", version: "5.19.9-200.fc36.x86_64", arch: "amd64", family: "unix"

Additional information

No response

[radcortez]

Set and List (and Arrays), follow the rules defined by the MP Config specification for empty values:
https://github.com/eclipse/microprofile-config/blob/2.0/spec/src/main/asciidoc/configexamples.asciidoc#config-value-conversion-rules

You can also have a look for a more in-depth discussion: eclipse/microprofile-config#446.

Map is not supported by MP Config. It was something that we added. It is, in fact, inconsistent with other types, but it also behaves a little differently than the rest. For instance, while keys on other types are well known, keys in Maps are dynamic, meaning that to populate the Map we have to iterate the available property names to match them with the Map. Other types are populated by selecting the property directly. We can probably validate the map at the end and fail, but we are not able to tell with property is missing, since the key is dynamic.

A change like this will be a breaking change, and while I'm always +1 for consistency, we need to be careful here.

@dmlloyd any thoughts?

[ctron]

Thanks for the explanation, that really helps to understand the background.

So if Set/List/Array can't be changed easily, and Map is an out-of-spec extension anyway. Why not just allow Optional<Map<>> in addition. I do agree that technically it doesn't add much, but from a consistency point of view, it does IMHO.

[radcortez]

I'll have a look.

[frehov]

I think we hit something similar to what is described here, but in Quarkus 3.1.1 with kotlin.
In short, we have multiple levels of nested properties.

If we use Map<String, String> in a complex type that is already annotated with @WithDefault on the parent field, that works.
If we add a field with List<String> it crashes with a notification about required property missing.
If we add a field with List<ComplexType> it does not crash, and gives us an ampty list instance.

The included example should demonstrates this behaviour,

@ConfigMapping(prefix = "nested")
interface NestedConfig {
    @WithDefault("")
    fun list(): List<Nested>

    interface Nested {
        fun ok(): Map<String, String>
        fun okAgain(): List<ComplexType>
        fun notOk(): List<String> // Comment out/remove this line/add properties, and app starts like normal

        interface ComplexType {
            fun name(): String
        }

    }
}

This "incosistent" handling doesn't exactly make sense to me.
Why should boxed primitives and String not resolve to an empty list, whereas the Complex type defined by an interface is allowed to?

I would also expect this to work: (using yaml-config)

nested:
  list:
    - not-ok: [] # This is considered as missing/unmappable

[radcortez]

This "incosistent" handling doesn't exactly make sense to me. Why should boxed primitives and String not resolve to an empty list, whereas the Complex type defined by an interface is allowed to?

This is because we comply with the MicroProfile Config specification. The specification, states the behavior for boxes primitives / String configuration for Arrays and Lists here: https://github.com/eclipse/microprofile-config/blob/master/spec/src/main/asciidoc/configexamples.asciidoc#config-value-conversion-rules. Both Map and complex type List, are only available in SmallRye Config, so no rules are specified for these cases.

I understand that it may be inconsistent with other properties. We could make both cases have the same behavior to other properties, but also get some questions we need to answer:

• How do we report back which map key / element index has to exist in the Map or List?

One additional point on inconsistency is how MicroProfile Config adds support for Arrays / Collections. There is only support for single elements, and the configuration values are set using a comma-separated string. Obviously, this is not suited for Collections of nested elements types. In this case, MicroProfile Config, does mandate the config property to exist, but due to the nature of how it is defined, it is easy to resolve and report to the user.