Converter for interface type is ignored

[AlexIvchenko]

I have library interface and two implementations which are useful for configuring my application e.g.:

interface Interface {
  static Interface of(long value) {
    return new LongImpl(value);
  }

  static Interface of(String value) {
    return new StringImpl(value);
  }

  long asLong();

  String asString();
}

class StringImpl implement Interface {...}

class LongImpl implement Interface {...}

I want to use Interface type as config property, e.g.:

@ConfigMapping(prefix = "test") 
interface Config {
  Interface prop();
}

• I register spi converter for interface:

class InterfaceConverter implements Converter<Interface> {
    @Override
    public Interface convert(String s) throws IllegalArgumentException, NullPointerException {
        return Interface.of(s);
    }
}

• And provide config:

test:
  prop: value

However, I am getting an error:

java.util.NoSuchElementException: SRCFG00014: The config property test.prop.as-long is required but it could not be found in any config source
java.util.NoSuchElementException: SRCFG00014: The config property test.prop.as-string is required but it could not be found in any config source

Looks like smallrye-config doesn't check if there is a registered converter for an interface and always consider interfaces as nested config mapping.

If I use a particular implementation of interfaces in config and converter, it's working:

@ConfigMapping(prefix = "test") 
interface Config {
  StringImpl prop();
}

class StringImplConverter implements Converter<StringImpl> {
    @Override
    public StringImpl convert(String s) throws IllegalArgumentException, NullPointerException {
        return new StringImpl(s);
    }
}

Documentation of microprofile-config doesn't mention that it's prohibited to use converters for interfaces and probably this functionality can be implemented.

[dmlloyd]

This seems like a legitimate bug to me. I should have added a check to see if there was a converter for the given type before assuming it was a group.

[radcortez]

Correct. Unfortunately, it is not that easy because the mapping introspection and generation (which assumes that the interface is a group), is independent of SmallRyeConfig (no access to the current runtime discovered / registered Converters), to determine if the interface is a group or if there is a converter registered for it.

Adding a @WithConverter(...) achieves the desired behavior, with the downside of adding the annotation.

I'm unsure if we should expose the runtime converters to the mapping generation. For instance, in Quarkus, all mappings are generated at build time. If the generation is dependent of the runtime configuration, that would mean that we cannot generate the mappings anymore (at least for runtime config). While unlikely, a converter could be registered for runtime config and it could shade an element of the mapping (a group in static config and a converted element in runtime config).

I can't think of a good way to properly choose between a group or a converter (without access to the runtime list, or metadata). Another related issue is the config processor to generate the docs. At first glance, I believe there is no way for us to reliably find which converters are going to be available at each phase, but I need to put more thought in this.

Any thoughts @dmlloyd?

[dmlloyd]

I think you are right: unless you know ahead of time which interfaces have a converter, you would need the explicit annotation.

Hypothetically you could replicate the discovery process of SmallRye Config, if we have defined it rigidly enough. For ServiceLoader-based converters this is could be fine using just what is in jandex. But implicit converter detection could easily give the wrong result, if (for example) there happens to be an of(String) method on the group interface (which is definitely not impossible).

I guess an explicit annotation for this case is needed. Maybe though we want to make the value argument optional for WithConverter in order to indicate that it can use an implicit converter instead of specifying an explicit one. At least this would make it slightly less onerous.

[radcortez]

Hypothetically you could replicate the discovery process of SmallRye Config, if we have defined it rigidly enough. For ServiceLoader-based converters this is could be fine using just what is in jandex. But implicit converter detection could easily give the wrong result, if (for example) there happens to be an of(String) method on the group interface (which is definitely not impossible).

Yes, I thought about this, but we also allow access to the config builder, meaning that Converter could be registered programmatically. Converter's discovery would always be very unreliable.

I guess an explicit annotation for this case is needed. Maybe though we want to make the value argument optional for WithConverter in order to indicate that it can use an implicit converter instead of specifying an explicit one. At least this would make it slightly less onerous.

Sounds reasonable.