Add ConverterProvider type to handle Converter retrieval for parameterized type

[NicoNes]

Description

Suppose that I create a parameterized type as follow:

public class MyObject<T> {

    ...

}

If I want to use this parameterized type in the following injection points

@Inject
@ConfigProperty(name = "property1")
private MyObject<Type1> property1;

@Inject
@ConfigProperty(name = "property2")
private MyObject<Type2> property2;

I have two provide a global converter or an implicit converter. But the problem with both of them is they won't tell nothing me about the type argument of the parameterized type at the given injection point.

Proposal

So my proposal would be to add a ConverterProvider type as follow to handle this case

public interface ConverterProvider {

    public <T> Converter<T> getConverter(Class<T> rawType, Type genericType);

}

Just like Converter this ConverterProvider would be discovered by SPI or provided programatically. Then it would be invoked when resolving injection points to retrieve the right Converter. If no ConverterProvider is available or if it does not provide no eligible Converter then the current Converter retrieval mechanism is applied.

The concept of ConverterProvider is taken form JAX-RS spec where the equivalent is ParamConverterProvider.

WDYT ? Am I missing something ?

Thanks

[Emily-Jiang]

I am unclear about your use case. As for your suggestion, it's unclear who implement the spi mentioned in your issue. Once you got hold of the converter, in what use case you are using it for?

[NicoNes]

Let me clarify my needs.

At properties injection point, I would like to be able to convert properties coming from my ConfigSource into a custom parameterized type like that:

@Inject
@ConfigProperty(name = "property1")
private MyObject<Type1> property1;

It seems to me that there is for now nothing in the spec allowing/describing how to do that.

The only option for now seems to be to create a type like following

public class MyType1Object extends MyObject<MyType1> {
}

Then create and implement a Converter<MyType1Object > and perform following injection instead of the initial one described above:

@Inject
@ConfigProperty(name = "property1")
private MyType1Object property1;

So to my mind this is a bit frustrating to not be able to take advantage of the generic type without being forced to subtype it.

To sum up, my proposal is to provide a simple way for user to convert properties into their own custom parameterized type in addition to the Array, List and Set that are by default supported according to the spec.

At this point am I wrong or am i missing something ?

As for your suggestion, it's unclear who implement the spi mentioned in your issue

The ConverterProvider spi is to be provided and implemented by the user

For my previous example it could be something like:

public class ConverterProviderImpl implements ConverterProvider {

    @Override
    public <T> Converter<T> getConverter(Class<T> rawType, Type genericType) {
        if(!MyObject.class.isAssignableFrom(rawType)) {
            return null;
        }
        Class<?> typeArgument = getTypeArgument(genericType);
        if(Type1.class.isAssignableFrom(typeArgument)) {
            return new MyConverter<MyObject<Type1>>();
        }else if(Type2.class.isAssignableFrom(typeArgument)) {
            return new MyConverter<MyObject<Type2>>();
        }
        return null;
    }

}

Once you got hold of the converter, in what use case you are using it for?

For all properties injection point discovered by the Mp-Config implementation, the Mp-Config implementation will have to invoke all ConverterProvider implementation (discovered or provided by user through the ConfigBuilder) one after other with the injection point raw type and generic type if any, until the first non null Converter is returned. If no non null Converter is returned then the existing current Converter retrieval mechanism is applied.

Is it clearer or am I missing something ?

Thanks

[ljnelson]

IMHO you're not missing anything. Your use case is highlighting a gap (perhaps) in the specification: the specification does not say anything about a Converter whose conversion type is a parameterized type.

Your proposal, though, as written, wouldn't handle something like Converter<List<List<Integer>>> (unless I've misread it). To do this sort of thing requires type resolution and recursive combing through ParameterizedType's actualTypeArguments and accounting for various edge cases as well as what to do if an exact converter isn't found.

My personal side project MicroProfile Config implementation does all this to a limited extent; you can see examples of it here: https://github.com/microbean/microbean-microprofile-config/blob/master/src/test/java/org/microbean/microprofile/config/TestConverters.java. The {holds nose} 😄 code that accomplishes this probably not as well as it should is here: https://github.com/microbean/microbean-microprofile-config/blob/master/src/main/java/org/microbean/microprofile/config/ConversionHub.java#L366-L487 It has been a while since I've looked at it so it should be used just as an example of how an implementation could do this, and not as an example of any best practices!

Note that this sort of thing doesn't require a separate interface. The specification would just have to require implementations to handle type resolution properly when discovering converters. Currently it does not.

[NicoNes]

Hey @ljnelson,

Happy to read that you spotted the same little "gap" in the specification.

Your proposal, though, as written, wouldn't handle something like Converter<List<List>> (unless I've misread it). To do this sort of thing requires type resolution and recursive combing through ParameterizedType's actualTypeArguments and accounting for various edge cases as well as what to do if an exact converter isn't found.

Actually my proposal handles this case too.

If the injection point is like:

@Inject
@ConfigProperty(name = "property1")
private List<List<Integer>>property1;

Then the MP-config implementation will invoke user provided ConverterProvider with List (field.getType()) as the raw type and List<List<Integer>> (field.getGenericType()) as the generic type. Then just like in my previous ConverterProviderImpl example it is up to the user to implement all required code to extract ParameterizedType's actualTypeArguments to return the correct Converter if any.

Note that this sort of thing doesn't require a separate interface. The specification would just have to require implementations to handle type resolution properly when discovering converters. Currently it does not.

I agree with you. But as you may have experiment with your side project, trying to produce a generic code to deal with all possible parameterized type injection cases can be really complex. So instead of asking MP-CONFIG vendors to do this complex generic code to handle all cases, it's easier to ask user to provide their specific code that works for their given case. So adding the ConverterProvider interface concept is more about allowing user to deal with Converter with parameterized type than asking to the MP-config vendors to do it for them.

Another thing in favor of the ConverterProvider is that this concept already exist in JAX-RS spec (see ParamConverterProvider) so maybe it will be great to preserve a sort of "consistency" between those specs.

[ljnelson]

Obviously either approach then works. I am always personally in favor of fewer constructs in a specification. Fewer things to maintain is usually better. Lastly, most MicroProfile vendors are already familiar with generic type wrangling since they have to work with JAXB and CDI; I don't think it's an undue burden to make them (us!) do this work (there's probably whole swaths of code that can be reused). That's just my opinion, of course.

[NicoNes]

Actually, our two approaches to fix this gap can both be part of the spec and work together.

I explain.

Suppose the following injection points:

@Inject
@ConfigProperty(name = "property1")
private MyObject<Type1> property1;

@Inject
@ConfigProperty(name = "property2")
private MyObject<Type2> property2;

To provide converters for those injection points, the developper can either

• Create and provide two Converter like:

public class MyType1ObjectConverter extends Converter<MyObject<Type1>> {
}

and

public class MyType2ObjectConverter extends Converter<MyObject<Type2>> {
}

and in this case your proposal fit perfectly.

• Create one Converter type whose instances will only differ in their type argument like:

public class MyObjectConverter<T> extends Converter<MyObject<T>> {
}

and then provide a ConverterProvider in charge of creating and returning instance of MyObjectConverter<T> like:

public class ConverterProviderImpl implements ConverterProvider {

    @Override
    public <T> Converter<T> getConverter(Class<T> rawType, Type genericType) {
        if(!MyObject.class.isAssignableFrom(rawType)) {
            return null;
        }
        Class<?> typeArgument = getTypeArgument(genericType);
        if(Type1.class.isAssignableFrom(typeArgument)) {
            return new MyObjectConverter<Type1>(...);
             }else if(Type2.class.isAssignableFrom(typeArgument)) {
            return new MyObjectConverter<Type2>>(....);
        }
        return null;
    }

}

The concept of having both Converter and ConverterProvider looks like existing ConfigSource and ConfigSourceProvider.

WDYT ?

[radcortez]

This seems very similar with the behaviour of javax.ws.rs.core.Response#readEntity(javax.ws.rs.core.GenericType<T>). Basically, its a conversion from a stream entity into a type as well and it uses a GenericType to pass additional information to help with the conversion.

Maybe we should consider using the same approach?

[NicoNes]

Yes, just like in JAX-RS this issue has two chapters:

• the first one is about handling configuration property conversion into parametrized type at injection points. (What we have been talking about for now)
• the second one is about handling configuration property conversion into parametrized type programmatically using config.getValue(propertyName, propertyType) or config.getOptionalValue(propertyName, propertyType).

About the "second chapter" I agree with you that GenericType like in JAX-RS should be used. This way user should be able to programmatically convert there properties into parameterized type doing: config.getValue("property1", new GenericType<List<Integer>>{}) or config.getOptionalValue("property1", new GenericType<List<Integer>>{}).

[Emily-Jiang]

Thanks @NicoNes for further explaining this! Let's agree on the use case:

As an end user, I want to have the Type parameter honoured on performing injection or programmantic lookup.

@Inject
@ConfigProperty(name = "property1")
private MyObject<Type1> property1;

MyObject<Type1> pro1=ConfigProvider.getConfig().getValue("property1", MyObject.class)

As for the solution to the above use case, for simplicity, we should put the support in the implementation like what @ljnelson suggested. I only expect the end user to provide a converter to convert String to MyObject. The implementation should do an exact search first if not found out. Then fallback to assignable operation.

[NicoNes]

@Emily-Jiang The use case are correct !

• The proposal of @ljnelson works fine for use case 1 (the one with injection) :+1:

• For use case 2 (programmatic lookup) it is not enough. Something is missing to pass additional information about the type argument requested by end user.

Actually when doing:

MyObject<Type1> pro1=ConfigProvider.getConfig().getValue("property1", MyObject.class)

The MyObject.class only tells the implementation that end user wants an instance of MyObject not MyObject<Type1>.

So my proposal based on @radcortez suggestion is to rely on either

• javax.enterprise.util.TypeLiteral<T> from CDI-api (will add a dependance to CDI-api)
• or custom type like GenericType<T> in JAX-RS (no dependance to CDI-api)

and add a method in Config class allowing end user to do:

MyObject<Type1> pro1=ConfigProvider.getConfig().getValue("property1", new TypeLiteral<MyObject<Type1>>{})

[radcortez]

We probably need to stay away from adding a direct dependency to CDI, so my preference would be to add a custom the like JAX-RS.

[Emily-Jiang]

I am not convinced whether we need to add TypeLiteral as yet. I think for the programmatic look, the runtime can get the type parameter from the field type. I think the Converter api needs to be updated to take in the Type Parameter, which was discussed in the past. I think with this change the use case should be fulfilled.

[NicoNes]

I think for the programmatic look, the runtime can get the type parameter from the field type

@Emily-Jiang I don't get your point. How it is possible ?

[jbee]

The Converter abstraction per se is capable of generic type conversion as its generic type isn't bound anywhere. A converter "just knows" how to produce a certain target type and the config internally could operate on Type level. It just isn't very convenient nor efficient to create converters for any full generic type combination. The crux currently is that even if a generic type converter would be defined you cannot get it out of the config or have it used as the Config API works on Class not Type. The obvious way to address this would be to overload getValue with:

<T> T getValue(String property, Type genericType);

This still allows to use JAX-RS GenericType in lookup like this

List<Integer> list = config.getValue("prop", new GenericType<List<Integer>>{}.getType())

but it also allows to use other helpers that provide Type instances like CDI's TypeLiteral which also has a getType() method.

So far this is fairly straight forward. One detail question might be whether to return T or Object.

The harder problem is how to make conversion fully generic. The proposed provider to me falls short as it does not allow composition based on the converters defined within the registry. Foo<Bar> would not necessarily be based on the Bar converter otherwise defined in the config context. Also it feels like a second alternative mechanism to provide converters with slightly different capability. I'd prefer to look into ways to extend the capabilities of the existing converters and their registration in a way that allow for composition and reuse of converters.

One aspect that has not been raised are upper bound types.This would also include ? extends List<?> to be a type you could provide a converter for which is capable producing any actual List type.

[ljnelson]

All of this. +1000. Java Language Specification-compliant type assignability using java.lang.reflect.Type is quite difficult but not impossible.

[NicoNes]

@jbee, about using this:

<T> T getValue(String property, Type genericType);

What happen if user invokes this method with its own Type implementation since nothing prevents him from doing it ?

I personnaly prefer :

<T> T getValue(String property, TypeLiteral<T> typeLiteral);

Except the fact that using TypeLitteral will add one more dependency, I think a solution like that could be the best option to solve the programmatic lookup issue. The TypeLitteral.getType() final method cannot be overriden by user so MP-Config vendors won't have to deal with all possible Type implementation.

EDIT: Object getValue(String property, Type genericType); will force user to use casts and does not allow compiler strong type checking. I'm personally not a huge fan of this one.

About all other points I agree with you and @ljnelson that MP-Config vendors can get the Type of the provided Converter and then do exact match on this Type to lookup the right Converter.

I said exact match because if user adds following Converter

private class Converter implements org.eclipse.microprofile.config.spi.Converter<List<?>>{
}

and ask for following conversion:

List<Integer> list = config.getValue("prop", new TypeLiteral<List<Integer>>{})

The Converter<List<?>> should not be invoked since we can't ensure that the type produced by this Converter<List<?>> is assignable from List<Integer>. So instead in this case an IllegalArgumentException should be thrown.

[jbee]

What happen if user invokes this method with its own Type implementation since nothing prevents him from doing it ?

Independent of how it is passed you cannot be sure where the underlying Type instances originates from. Implementations always have to assume it could also be a user defined class. You could for example simply use https://google.github.io/guice/api-docs/3.0/javadoc/com/google/inject/TypeLiteral.html#get(java.lang.reflect.Type) to create the a TypeLiteral from your "own" Type instance.

This usually isn't a problem (and you will find that there are many implementation classes for the Type interface family already in common libraries) as they implement the relevant methods based on the interface they represent. And should a user defined a Type implementation improperly with regards to the relevant methods this isn't a failure of the Config implementation but the user defined type.

[NicoNes]

Well I'm not sure you get my point.

As a user I can create this valid following Type implementation in regards to the relevant method provided by the Type interface right ?

public class MyType implements Type {
  public String getTypeName(){
      return "MyType";
  }
}

If I use it to invoke <T> T getValue(String property, Type genericType); it will fail because the MP-Config implementation can handle my valid custom Type implementation. It is not a failure from the user right ?

So to not have to deal with this kind of problem my proposal is to rely on javax.enterprise.util.TypeLiteral<T> from CDI. This javax.enterprise.util.TypeLiteral<T> cannot be used to wrap any custom Type implementation contrary to the one provided by Guice and contrary to JAX-RS GenericType<T>. So it ensures you that the Type instance returned by the getType() will always be a JVM native one not a user custom one.

So it just my opinion but I think it's one of the reaon why Type is not used in CDI public method signature API and why the specific javax.enterprise.util.TypeLiteral<T> is used instead. It would be interresting to check if other JAVA APIS use Type in their public method signature or not. JAX-RS does not and provided GenericType<T> instead but as explianed above this GenericType<T> can't guarantee you that the returned Type is a JVM native one.

Am I missing something ?

[jbee]

Am I missing something ?

If javax.enterprise.util.TypeLiteral would be used in the signature the Type instances provided by a injection points cannot be made into a TypeLiteral and CDI would need to be implemented using proprietary API of the Config implementation that is based on Type again. The gab between what CDI requires to be implemented and what the public Config API actually allows doing would further grow instead of disappear as attempted by other PRs.

it will fail because the MP-Config implementation can handle my valid custom Type implementation. It is not a failure from the user right ?

Either Type got implemented in a valid way then this does not cause any failure (equals and hashCode are most essential) or they aren't implemented in a valid way which is a mistake made by the user and naturally incorrect input would be unlikely to yield the desired output. I really don't see why this parameter is so special. Otherwise one could equally argue that the property parameter should never be a String as I could pass values that would never be allowed as a property in a property file.

[NicoNes]

Well sorry guys, I deleted my previous answer because it tooks me long to get your point @jbee about your following statement but I think I get it now:

If javax.enterprise.util.TypeLiteral would be used in the signature the Type instances provided by a injection points cannot be made into a TypeLiteral and CDI would need to be implemented using proprietary API of the Config implementation that is based on Type again. The gab between what CDI requires to be implemented and what the public Config API actually allows doing would further grow instead of disappear as attempted by other PRs.

So if my understanding is right, you are saying that javax.enterprise.util.TypeLiteral in the signature is not a good choice because javax.enterprise.util.TypeLiteral instance cannot be instantiated to wrap Type retrieved at injection point in order to invoke the method <T> T getValue(String property, javax.enterprise.util.TypeLiteral<T> genericType) right ?

If my understanding is right I agree with you on this one.

Otherwise one could equally argue that the property parameter should never be a String as I could pass values that would never be allowed as a property in a property file.

I agree with you here too. If the provided Type is not valid or not handled by MP-Config implementation an exception should simply be thrown .

But I still got a problem with your signature proposal <T> T getValue(String property, Type genericType)

It does not enforce type inference at all in all cases.

It works fine for the following statetement:

List<Integer> list = config.getValue("myproperty", new TypeLiteralWhatever<List<Integer>>(){}.getType());

But it does not automatically works for following one and requires user to use type witness (<List<Integer>>):

config.<List<Integer>>getValue("myproperty", new TypeLiteralWhatever<List<Integer>>(){}.getType()).get(1)

So unless I am wrong about the type inference issue, I think the MP-Config spec should globally tries it best to ensure type inference when possible.

So I think that a custom TypeLiteral<T> (like JAX-RS GenericType<T>) allowing to wrap any Type, could be defined by MP-Config spec an used in signature as follow:

<T> T getValue(String property, ConfigTypeLiteral genericType)

This could be a good compromise to solve both the issue you spotted in my proposal with javax.enterprise.util.TypeLiteral and the type inference problem of yours since no type witness will be required in this case:

List<Integer> list = config.getValue("myproperty", new ConfigTypeLiteral<List<Integer>>(){}); or config.getValue("myproperty", new TypeLiteralWhatever<List<Integer>>(){}).get(1)

WDYT?

[jbee]

In summary, as @NicoNes points out the downside of Type in the signature is loss of type safety when used programmatically. While javax.enterprise.util.TypeLiteral does enforce type safety it does not allow to use the Type instance extracted via reflection as it is the case for injection points. In-between these two is using some wrapper type with a generic (like GenericType) which does allow wrapping a Type instance (unsafe) but also can be used type safe but cannot enforce it.