GenericConversionService should match target converters for compatible subclasses [SPR-8675]

[spring-projects-issues]

David Smiley opened SPR-8675 and commented

I registered this simple Converter (which by the way, I feel is something that should be out of the box):

public class FileToResourceConverter implements Converter<File,Resource> {
  public Resource convert(File source) {
    return new FileSystemResource(source);
  }
}

I have code that would like to look up a converter for an InputStreamSource (a super-interface of Resource):

InputStreamSource rsrc = this.getConversionService().convert(message.getPayload(), InputStreamSource.class);

But this fails because, apparently, GenericConversionService is unable to recognize instanceof relationships between the conversion target type and the desired target type. As a workaround, my code here has to be aware that the converter is for Resource and then ask the conversion service for a Resource even though it only needs an InputStreamSource.

---

Affects: 3.0.6

Issue Links:

• ClassCastException possible when converting to a target type that is a subtype of T for a Converter<S, T> convertible type. [SPR-8718] #13360 ClassCastException possible when converting to a target type that is a subtype of T for a Converter<S, T> convertible type.
• INT-3556 Use Converters that can convert to subtype of datatype channel

0 votes, 7 watchers

[spring-projects-issues]

Keith Donald commented

This is expected behavior given the design of the conversion system. You're asking the ConversionService to convert a File to a InputStreamSource, but you haven't registered a conversion rule for this pair, so you're getting a ConverterNotFoundException. Rather, you've registered the FileToResource convertible pair, which is considered a more specific conversion rule. You can register a broader rule by calling ConversionService#addConverter(File.class, InputStreamSource.class, fileToResourceConverter).

[spring-projects-issues]

Keith Donald commented

We could consider registering these broader rules for you when a converter is added to the registry... for example, on addConverter(new FileToResourceConverter()) we could at that time traverse Resource's type hierarchy and index the converter at those higher levels (File->InputStreamSource in this case). We need to think this through though to make sure there would be no side effects. Right now, as outlined in my previous comment, you need to register the more general conversion rule explicitly.

[spring-projects-issues]

Keith Donald commented

Consider now for example we have GenericConverters registered for broad types such as ArrayToCollection. This enables one to take any array and convert it to another java.util.Collection type, where the GenericConverter is passed the desired targetType, such as a List, Set, or other specific extension/implementation. Having a single "collection converter" for the entire hierarchy is nice, as it's a single converter class to maintain.

We wouldn't want to lose or break that capability. If someone came along and registered an ArrayToMySpecialList converter, we would not want to overwrite the more general ArrayToCollection conversion logic (trying to bind an Array to a Collection field and getting a MySpecialList instead of a regular ArrayList might be unexpected). That's a side effect I'd be worried about if we introduced an algorithm that traverses the hierarchy of the target type and indexes the provided converter at each level up. We would likely have to detect if a converter is already registered at a given level, and if so, stop. Then the order in which you register your Converters could become more important, which might make things more confusing.

I'm interested in your thoughts.

[spring-projects-issues]

Rossen Stoyanchev commented

... apparently, GenericConversionService is unable to recognize instanceof relationships between the conversion target type and the desired target type

It would be tricky to use instanceof in this case. If Converter<File,Resource> and Converter<File,ContextResource> were both registered, it's not clear which one should be used? Order of registration might end up deciding it and that is a bit too subtle. I think it's appropriate to be explicit about what should be used when a desired type is more general than the registered types rather than relying on the order of registration. I.e.

ConversionService#addConverter(File.class, InputStreamSource.class, fileToResourceConverter);

[spring-projects-issues]

David Smiley commented

I think telling spring users they need to register their converters multiple times is reasonable, but spring should make it easier to do that. I am using ConversionServiceFactoryBean in my spring beans XML and I simply refer to my converter. Presumably this factory beean introspects my converter to see the covariant return type (Resource, in my case). Perhaps adding an annotation to the method could allow me to specify additional types.

I like the idea of auto-registering inherited types in the hierarchy upon adding the converter, letting the first one registered "win". I acknowledge that means the order of registration can be significant but that strikes me as an edge case not to be terribly concerned about.

I agree doing "instanceof" check at lookup time is probably not a good solution.

[spring-projects-issues]

Christopher Smith commented

I'd like to bump this issue and speak in support of matching subclasses on the target type. I'm trying to set up a Spring Integration Datatype channel with an abstract channel type and Converters that can convert various input payload types into concrete classes for the channel, but I can't configure the channel with just the superclass type because it won't match for conversion. I would expect, e.g., that a Converter<String,Double> would be acceptable for a conversion to Number.

[chrylis]

Amusingly/frustratingly, I find myself once back at this same issue; while integrating with Spring Security's PermissionEvaluator (which expects an ID of type Serializable), I found that providing a Converter<Entity, String> would not work, which is Surprising Behavior.