MP-Config 1.2.1: CDI runtime lookup of converted configuration values?

[Laird Nelson]

As a MicroProfile-Config implementor, I want (ideally) to support the case where my user does:

someInstance.select(Frobnicator.class, myConfigPropertyLiteral).get();

(That code was typed off-the-cuff and may contain errors; I hope you get the idea.)

If conversion were mandated to always occur through the Converter<T> interface, then to support this case and to validate it all I would have to do would be to gather up all SPI-discovered Converter<T> instances, find what T is, and, for every such type hierarchy, set up a dynamic producer method or bean returning objects of that type to satisfy that lookup.

But conversion is not mandated by the specification to always occur through the Converter<T> interface.  It can of course happen implicitly for any type that happens to be present on the classpath that has, for example, a valueOf(String) method.  That means I can't enumerate all possible conversion types; that, in turn, means I can't register beans of those types; that, finally, means I can't support runtime lookup of arbitrary conversion types.

So if the user does this above, she will get a runtime error, even though if she did @Inject @ConfigProperty("productionUrl") private Frobnicator url; it would work (assuming either a Converter<Frobnicator> or a Frobnicator#valueOf(String) method).

(Also the specification never says what the Config#getValue(String, Class) method is obliged to do, incidentally; so conversion in general is underspecified, unless I'm missing something.  I understand that the specification wants you to use Converters when implementing this method, but I'm not sure it actually prescribes anything in this area.)

So if I'm not missing anything major, it looks like runtime lookup of arbitrary configuration values in CDI can't be satisfied in the "CDI way": i.e. at CDI-portable-extension-AfterDeploymentValidation-time, we can't know what the user might do later at runtime: the lookup might work, if we happen to have a bean or producer registered to support it, or not, if we don't.  I'm assuming this is probably why the specification requires that a Config be able to be injected or looked up in a CDI environment, so that she can kind of "opt out" of CDI when she needs to do runtime lookups and other sorts of non-deterministic actions and just use the Config APIs directly.

Is all of this more or less a correct understanding of the situation?

Best,

Laird

[Ondro Mihályi]

Hi Laird,

> Also the specification never says what the Config#getValue(String, Class) method is obliged to do

This is intentional, because it depends on how the Config instance is created. If you use a builder to create it, default converters need to be turned on with ConfigBuilder.addDiscoveredConverters, otherwise default convetersare are disabled.

If Config is injected, it's mandated that it should be the same as the one from

ConfigProvider.getConfig().

However, it's not explicitly stated that the default config should enable discovered converters. I believe this is covered by tests in the TCK, but to clarify this I've raised an issue: https://github.com/eclipse/microprofile-config/issues/348

[Ondro Mihályi]

To comment on the main issue:

> So if I'm not missing anything major, it looks like runtime lookup of arbitrary configuration values in CDI can't be satisfied in the "CDI way"

I believe you're correct unless there's a more flexible way to define producers when they are needed. I beleive that currently most of the implementations create producers during deploy time - they discover all injection points in CDI beans and create a producer for them. This clearly doesn't create producers for types which are only injected dynamically as in your example.

A workaround for a developer is to inject a config value of type Frobnicator once into any CDI bean so that a producer is created during deployment.

If anybody finds out a way how to create producers dynamically on demand it would be worth to let others know. We could then create some tests to cover this scenario and guide implementers how to implement it.

--Ondro

On Monday, April 16, 2018 at 10:57:17 PM UTC+2, Laird Nelson wrote:

[Laird Nelson]

On Thursday, April 19, 2018 at 6:08:27 AM UTC-7, Ondro Mihályi wrote:

> Also the specification never says what the Config#getValue(String, Class) method is obliged to do

This is intentional, because it depends on how the Config instance is created. If you use a builder to create it, default converters need to be turned on with ConfigBuilder.addDiscoveredConverters, otherwise default convetersare are disabled.

My point was more: from a specification standpoint, as far as I can tell this implementation of Config#getValue(String, Class) is legal:

@Override

public <T> T getValue(final String name, final Class<T> type) {

  // use java.beans.PropertyEditor or something else to perform conversion

  // e.g. no Converter<T> used here at all, let's say

  return result; // or just return null

}

…e.g. there's no restriction of any kind on how conversion should be done at all by this method, which means that the specification's talk about converters becomes irrelevant.  If there is a requirement that conversion be accomplished in some particular way, I think it needs to be expressed as a constraint on the implementation of this method.  Right now such a constraint does not actually exist, unless I've overlooked it (always possible).

Again, as a sympathetic reader I can infer that I should probably use the conversion mechanism discussed in the specification but I don't think there's a requirement that I must do so.

Best,

Laird

[Laird Nelson]

On Thursday, April 19, 2018 at 6:15:01 AM UTC-7, Ondro Mihályi wrote:

If anybody finds out a way how to create producers dynamically on demand it would be worth to let others know.

The closest I've found is from a blog entry I wrote while back: https://lairdnelson.wordpress.com/2017/02/11/dynamic-cdi-producer-methods/

Still not quite enough for the case I've outlined above, but only because with implicit conversion you don't have a way to determine, in advance, all the conversion types you might need.  Basically, MicroProfile-Config's CDI integration must happen at runtime for many different cases, which means you have the possibility of runtime type conversion failure.

For completeness, if all conversion were mandated to go through some typed mechanism beforehand (i.e. if all conversion were mandated to go through registered Converters, which of course has its own drawbacks), then you could detect all problems at deployment time because you could discover all maximally specific conversion types at deployment time.  Probably not worth the tradeoff but worth pointing out at any rate:the extent to which you don't require this is the same extent to which you permit runtime type conversion failures.

In the current world, it also just occurred to me rather hazily that even with scanning injection points—i.e. not just with strange, non-typical programmatic Instance lookups—you still might not have what you need (not entirely sure about this).  An injection point might be, for example:

@Inject

@ConfigProperty(name = "frobnicationInterval")

private final Number frobnicationInterval; // note: Number, not Integer, not Long, not Double….

…and although this is an utterly contrived example I'm not sure it would work, right?  Could I generate a producer for this?  Ultimately I'd do config.getValue("frobnicationInterval", Number.class)…and what would happen?  Maybe this is covered somewhere; not sure.

My inarticulate point here is that, in CDI, beans (particularly producers) typically "return"/produce maximally concrete types and then CDI type-matches the maximally specific concrete producer or bean with all the possible normally more abstract supertype injection points.

So you produce Long, but you inject Number.  You produce HashSet<Integer>, but you inject Collection<? extends Number>.  And so on.  That's not how CDI integration in MicroProfile-Config works.  Which is fine, BTW—just worth noting and looking at.  It just means that MicroProfile-Config CDI integration is one of those cases where not all potential failures can be detected at deployment time.  Might be worth calling out somewhere.

Best,

Laird 

[Laird Nelson]

On Thursday, April 19, 2018 at 11:54:15 AM UTC-7, Laird Nelson wrote:

In the current world, it also just occurred to me rather hazily that even with scanning injection points—i.e. not just with strange, non-typical programmatic Instance lookups—you still might not have what you need (not entirely sure about this).  An injection point might be, for example:

@Inject

@ConfigProperty(name = "frobnicationInterval")

private final Number frobnicationInterval; // note: Number, not Integer, not Long, not Double….

…and although this is an utterly contrived example I'm not sure it would work, right?  Could I generate a producer for this?  Ultimately I'd do config.getValue("frobnicationInterval", Number.class)…and what would happen?  Maybe this is covered somewhere; not sure.

(I should mention for completeness that at least when you're talking about injection points you can validate them in AfterDeploymentValidation events, so you can catch a subset of type conversion errors here.)

Best,

Laird

[Ondro Mihályi]

I've clarified this in this pull request: https://github.com/eclipse/microprofile-config/pull/353/files

Please review and comment.

Ondro

[Ondro Mihályi]

I think that the case with injecting Number would be automatically raised as a problem by every CDI container. There are default converters for Float, Double, Integer, etc. and all of them could be injected at the injection point. If there's a producer for each type (which I assume every MP Config implementaion does) then there are multiple producers that could inject a vlue there and the injection point is ambiguous.

Ondro

[Ondro Mihályi]

Hi,

> The closest I've found is from a blog entry I wrote while back: https://lairdnelson.wordpress.com/2017/02/11/dynamic-cdi-producer-methods/

This is indeed not a good enough solution for truly dynamic config retrieval of values using implicit converters.

Supporting retrieval of

someInstance.select(Frobnicator.class, myConfigPropertyLiteral).get();

in a portable way isn't possible AFAIK. It's only possible if using implementation-specific API (provided by Weld, OpenWebBeans, etc.) to hook into the Instance implementation and provide beans during lookup if the API allows it. Or the CDI impl would have to be patched, which might be OK for application servers but not OK for libraries that intend to work with stock CDI implementations.

Until this is possible to support portably, we can clarify in the spec that support for this is optional. Applications that need to be portable should either provide converters also for types with implicit converters or inject some value for every used implicit type using Instance, e.g.:

@Dependent
public class RegisterImplicitConverters {
  @Inject
  Instance<Frobnicator> registerFrobnicatorImplicitConverter;
}

When the type is used in an injection point, implementations should discover it and create a producer for that type.

Ondro

[Laird Nelson]

On Friday, April 20, 2018 at 5:49:41 AM UTC-7, Ondro Mihályi wrote:

I think that the case with injecting Number would be automatically raised as a problem by every CDI container. There are default converters for Float, Double, Integer, etc. and all of them could be injected at the injection point.

Yes, for sure.

My point was because you really have no choice but to create a new Bean for each injection point dynamically, then at the time of Bean creation, you the CDI extension author hopefully don't know anything specific about the type in question.  So you see that the type of the injection point is, say, Number, and your code simply asks for config.getValue("name", Number.class).  That is, instead of getting an error that says, effectively, "There are already a bunch of beans/producers that can make this value for you", you get an error saying "Can't find a converter for type Number".  That's different.

(Obviously it's still an error and in general therefore there's no way to inject @ConfigProperty private Number someNumber anyway.)

Best,

Laird

[Laird Nelson]

On Friday, April 20, 2018 at 6:48:08 AM UTC-7, Ondro Mihályi wrote:

Supporting retrieval of

someInstance.select(Frobnicator.class, myConfigPropertyLiteral).get();

in a portable way isn't possible AFAIK.

OK; as I thought.  It would be if all type conversion was required to go through Converter instances.  (I'm not arguing that that is worth the tradeoff.)

Best,

Laird 

[Laird Nelson]

On Friday, April 20, 2018 at 5:42:41 AM UTC-7, Ondro Mihályi wrote:

I've clarified this in this pull request: https://github.com/eclipse/microprofile-config/pull/353/files

Please review and comment.

I added a couple of comments over there.

In general I think the specification and hence the javadoc language should be very explicit in terms of what you can and cannot do, because CDI is so wiggly.  Right now IMHO as an implementor it is not (I can obviously figure out what was intended, but that's not what we're talking about here).

So if @Inject @ConfigProperty private Number someNumber and other classes of this problem are forbidden, say so.  If programmatic lookup is impossible, say explicitly that that behavior is not guaranteed.  If @Inject @ConfigProperty private Collection<? extends String> strings is non-deterministic, or is allowed, say so explicitly.  Maybe it's simplest to identify the cases that are guaranteed to work, and to issue a blanket statement of undefined behavior for all other cases.

Thanks for your work here and willingness to accept feedback.

Best,

Laird

[Ondro Mihályi]

I agree it's good to rather overspecify than leave gaps in the spec. I'll try to add all clarifications into the PR. You are also free to raise a PR if you find something missing. To accept contributions from you we only need you to sign the Eclipse Foundation Contributor Agreement.

MicroProfile and Eclipse is all about feedback and collaboration. So we're glad for your feedback :)

--Ondro

--
You received this message because you are subscribed to a topic in the Google Groups "Eclipse MicroProfile" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/microprofile/3kXLoA1KVz8/unsubscribe.
To unsubscribe from this group and all its topics, send an email to microprofile+unsubscribe@googlegroups.com.
To post to this group, send email to microp...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/microprofile/7c446b99-2d36-4190-a5e3-446756bb132a%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Alasdair Nottingham]

I guess it depends on how it is written whether or not I’d be ok with this. 

The Number class doesn’t have a valueOf method so I wouldn’t expect to be able to inject a Number given MP Config 1.2. Perhaps the spec should state that the type conversion is based on the target type and not any super class or interfaces? 

Alasdair

You received this message because you are subscribed to the Google Groups "Eclipse MicroProfile" group.
To unsubscribe from this group and stop receiving emails from it, send an email to microprofile...@googlegroups.com.

To post to this group, send email to microp...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/microprofile/CACZTZYX1h2dnEjhJM9LAKCYmjUx%3DcBV2gZb%2B_3R17JU1_eBvhw%40mail.gmail.com.

[Mark Struberg]

Or introduce support for @Typed? Might also easily be overkill though, we need to think that through.

I can only explain what we do in geronimo-config. It respects the spec, but that dosn't mean that there are no other ways to interpret the spec.

We basically have a Map<Type, Converter>

This is initially populated with all the default converters and ones picked up via ServiceLoader.

At runtime if you request a conversion from String to Y, then we lookup if we already have a converter for Y. 

In case Y has a valueOf, it gets registered as Converter<Y> and ONLY for Y!

So even if Y extends X, then we do _not_ register Y.class as converter for X!

If you want to inject X and X doesn't provide any valueOf, then you have to provide a manual Converter for it.

E.g. for an InjectionPoint with type Number you probably have to register an explicit Converter<Number> which internally creates a java.lang.Double.

I'm not sure whether I personally would _ever_ inject a Number instead of a more concrete type though ;)

LieGrue,

strub

Am Montag, 23. April 2018 18:07:17 UTC+2 schrieb Alasdair Nottingham:

I guess it depends on how it is written whether or not I’d be ok with this. 

The Number class doesn’t have a valueOf method so I wouldn’t expect to be able to inject a Number given MP Config 1.2. Perhaps the spec should state that the type conversion is based on the target type and not any super class or interfaces? 

Alasdair

On Apr 20, 2018, at 7:23 PM, Ondro Mihályi <ondrej....@gmail.com> wrote:

I agree it's good to rather overspecify than leave gaps in the spec. I'll try to add all clarifications into the PR. You are also free to raise a PR if you find something missing. To accept contributions from you we only need you to sign the Eclipse Foundation Contributor Agreement.

MicroProfile and Eclipse is all about feedback and collaboration. So we're glad for your feedback :)

--Ondro

2018-04-20 23:55 GMT+02:00 Laird Nelson <ljne...@gmail.com>:

On Friday, April 20, 2018 at 5:42:41 AM UTC-7, Ondro Mihályi wrote:

I've clarified this in this pull request: https://github.com/eclipse/microprofile-config/pull/353/files

Please review and comment.

I added a couple of comments over there.

In general I think the specification and hence the javadoc language should be very explicit in terms of what you can and cannot do, because CDI is so wiggly.  Right now IMHO as an implementor it is not (I can obviously figure out what was intended, but that's not what we're talking about here).

So if @Inject @ConfigProperty private Number someNumber and other classes of this problem are forbidden, say so.  If programmatic lookup is impossible, say explicitly that that behavior is not guaranteed.  If @Inject @ConfigProperty private Collection<? extends String> strings is non-deterministic, or is allowed, say so explicitly.  Maybe it's simplest to identify the cases that are guaranteed to work, and to issue a blanket statement of undefined behavior for all other cases.

Thanks for your work here and willingness to accept feedback.

Best,

Laird

--
You received this message because you are subscribed to a topic in the Google Groups "Eclipse MicroProfile" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/microprofile/3kXLoA1KVz8/unsubscribe.

To unsubscribe from this group and all its topics, send an email to microprofile...@googlegroups.com.

To post to this group, send email to microp...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/microprofile/7c446b99-2d36-4190-a5e3-446756bb132a%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.