expose conversion mechanism in API

[emattheis]

There are certain cases where a user of microprofile-config might want to convert a supplied string into different type using the same mechanism as a resolved property value. For example, when a fallback value for a particular usage point is available from a string supplier, but not appropriate to contribute as a formal config source.

I suggest adding the following method to org.eclipse.microprofile.config.Config:

<T> T convert(String value, Class<T> type);

[dmlloyd]

In SmallRye Config, we expose that functionality like this:

    public <T> T convert(String value, Class<T> asType) {
        return value != null ? getConverter(asType).convert(value) : null;
    }

    public <T> Converter<T> getConverter(Class<T> asType) {
        // ...
    }

Which can be useful if one wants to cache the converter or perform repeated conversions, or create delegating converters, etc.

We also (internally) support getOptionalConverter which might be of use in the general API (yielding a Converter<Optional<T>>. But I'm not sure every implementation can support Optional using the converter infrastructure like we do.

[Emily-Jiang]

I know converter is a useful functionality. I talked with someone else to potentially create a converter spec. Since it is a smallish update, I'll attempt to propose a PR soon.

[Emily-Jiang]

Actually, I thought about this more yesterday and I'm not sure about adding the convert on the config api. It should not belong to Config object, which just a collections of configuration.

My proposal is to is directly expose Converter. I think we can do:

@Inject Converter converter;
MyType t = converter.convert("thisString");
#Programmtic way to look up converter
Converter converter = ConfigProvider.getConverter();
converter.convert("thisString");

A PR will follow soon.

[dmlloyd]

Actually, I thought about this more yesterday and I'm not sure about adding the convert on the config api. It should not belong to Config object, which just a collections of configuration.

This statement is incorrect. The Config object clearly associates both converters and configuration sources on a per-configuration basis as can be trivially extrapolated by the methods on ConfigBuilder. The original method as proposed is more correct, especially given that custom converters can be associated with a configuration. Your proposed mechanism loses this completely, and also doesn't actually work (there's no way to specify the target type in your programmatic approach).

[emattheis]

I agree with @dmlloyd. Config is the primary interface and serves to collect sources and converters behind a single abstraction. Even if we want to provide direct access to Converter, I’m not sure how your proposal would work without more explicit type information.

Personally, I’d like to see clearer separation between API and SPI so I think we should not leakConfigSource and Converter instances from Config, but that ship may have already sailed...

[Emily-Jiang]

This statement is incorrect. The Config object clearly associates both converters and configuration sources on a per-configuration basis as can be trivially extrapolated by the methods on ConfigBuilder. The original method as proposed is more correct, especially given that custom converters can be associated with a configuration. Your proposed mechanism loses this completely, and also doesn't actually work (there's no way to specify the target type in your programmatic approach).
Converter and ConfigSource can operate independently. A config object does not need to be constructed if only converter is used to convert a string, for an instance.

[kenfinnigan]

+1 to the PR as it stands.

Config has been used to date, rightly or wrongly, as a front for ConfigSource and Converter. This change simply continues that trend and aligns with previous work.

Exposing Converter directly would lead to more confusion and complexity as it becomes a user-facing API as opposed to an SPI for implementors.

I don't think you'd want to use a converter within Config unless you already had an existing Config instance present, so I'm not sure what your argument is there.

[dmlloyd]

Converter and ConfigSource can operate independently. A config object does not need to be constructed if only converter is used to convert a string, for an instance.

Configuration converters are primarily used in the context of config. If you're trying to get to having a general purpose conversion framework, that is something else altogether, and would probably have different requirements and uses (and doesn't belong in the MP Config specification at any rate - perhaps it would be worth a new MP specification for that purpose, if the scope & use cases could be identified).

[emattheis]

I would argue that the specified use of converters in microprofile-config is always in the context of a config. Multiple converters can exist for a given type and the selection of which converter to use depends on the converters known to a config and their respective priorities. Having access to the specific converter selected by the config for a given type could certainly be useful, so I like the SmallRye approach.

[emattheis]

to expose converters along with the ability to directly convert, I propose the following additions to Config:

default <T> T convert(String value, Class<T> type) {        
    return value == null ? null
                         : getConverter(type).orElseThrow(() -> new IllegalArgumentException("no converter available for " + type))
                                             .convert(value);
}

<T> Optional<Converter<T>> getConverter(Class<T> forType);

Essentially the same thing as SmallRye but with an exception-free way to indicate the absence of a suitable converter.

[dmlloyd]

It looks reasonable to me - although I don't like that our implementation will become incompatible; maybe a method name change would be possible?

[emattheis]

Yeah, I noticed the name collision with SmallRye, also. I took at a stab and reworking the SmallRye implementation and it doesn't seem too onerous to rename the existing SmallRye method, but of course that would be a breaking change to anyone using the SmallRyeConfig API directly.

I'm not necessarily opposed to a different name, but getConverer feels like the most appropriate choice.

[Emily-Jiang]

We can talk about this in the next release. I think separate Convert with Config property is better, so that coverter can be easily used by other spec for strict convesion usage.

[dmlloyd]

Holding this change for some future global conversion spec is not a wise course. If such a specification does ever materialize, only then should we consider applying it to this specification. In the meantime, we should assume that no such specification exists and proceed on that basis. Blocking useful features because a new specification might appear is not responsible stewardship IMO.

So, I disagree: let's address this for 1.5 or 2.0 if we can't get it in to 1.4.

[Emily-Jiang]

I have the following reasons that this issue is not in the scope of 1.4.

• I don't see end users need to directly use this to convert a string to an object. Besides it is odd to use this method to convert an arbitrary string to an object. I understand we might introduce a method withDefault(Object) for configValue.

• Since we are going to do some major work on the API in the next relese, we should wait instead of adding and then deleting shortly.

• The release train is departing. As you know, we don't even have time to do the null injection. Adding one API is just the starting. A lot of TCKs need to be written and then executed. We have no time.

• I am doing RC3 now and then impl needs to say they can pass TCK 100% and then we can produce final release by Monday/Tuesday to be included in the umbrella train.
  Hope this explains!

[emattheis]

My primary motivation for this issue is to provide a way for a user to programmatically convert a default value known to the application in the same way the injection facility does.

I find it very strange that this and #496 are meeting such resistance. I am essentially just looking for feature parity between Config and @ConfigProperty. Would it be helpful if I opened a new issue that addresses that disconnect directly?

Whether or not this can wait for the next release depends a lot on when that release is expected to happen. It's been 20 months since 1.3, and I had no idea the release train for 1.4 was departing until RC1 and RC2 arrived back to back. How do I learn more about the schedule for releases?

I can appreciate the pressures and challenges of running the project, but as a user and proponent of MicroProfile I'm trying to balance my desire to remain compliant with the standard for portability against the convenience offered by alternatives. I know I'm not alone with this challenge so i want to help push things along if I can.

[radcortez]

I agree that at this time it is hard to include additional features to 1.4. Considering the amount of discussion and back and forward with the null issue, I see that this particular one going on the same path and unfortunately there is no time for it if we want to get something out.

I also agree that we need to have a more clear roadmap, so contributors are aware of the release plan so they are able to push and advocate for the features they need to be included in the release cycle.

[Emily-Jiang]

@emattheis If you check the due date on milestone, you will find out the target date. The release train is departing. As for last year, we spent a lot of time discussing dynamic aspect of Config and created APIs. However, this is a great fit for Quarkus, which is static. We then undid a lot of stuff. We now take everyone onboard and align with each other. As @radcortez said, we will get the Config 1.4 out and then discuss what we want to focus on 2.0 so that we won't run into the same issue as last year. I understand your frustration. Hope we can move along faster after 1.4 release!

[emattheis]

If you check the due date on milestone, you will find out the target date.

Got it. We should get his linked up on the readme or contribution docs so it's clear.

Onward to 2.0!

[dmlloyd]

This should be included in 2.0 at least. There's simply no reason not to.

[radcortez]

MP-JWT requiring a Converter spec: microprofile/microprofile-jwt-auth#100

[radcortez]

How should we proceed with this for 2.0? Should we just expose some of the Converter method in Config?

[dmlloyd]

I asked @emattheis to rebase #495; once he does, @Emily-Jiang, could you please review it again from the perspective of possible inclusion in 2.0? @radcortez you could review too if you wish.

[emattheis]

I rebased the PR.

Also wondering if we should drop the convert method and just rely on getConverter to provide that functionality.

[radcortez]

This issue can now be closed, since #495 was merged.