orika-mapper / orika

Simpler, better and faster Java bean mapping framework
http://orika-mapper.github.io/orika-docs/
Apache License 2.0
1.29k stars 268 forks source link

CustomConverter interface to object #171

Open foal opened 7 years ago

foal commented 7 years ago

I have CustomConverter that declared as

public class UserConverter extends CustomConverter<IUser, User>

when I tried to map public class UserDto extends DomainDto implements IUser to User object the convertor didn't apply because canConvert method returns false (this.sourceType.equals(sourceType) is false)

The converter works in 1.4.6 release and stop works in 1.5.0 release.

foal commented 7 years ago

As workaround; change the converter from interface to implementation

public class UserConverter extends CustomConverter<UserDto, User>

ptahchiev commented 7 years ago

Hi @foal could it be something to do with this:

https://github.com/orika-mapper/orika/commit/f2193ee8d654c5ae067fb0b7c4c6ca63df5a2a74#commitcomment-20145561

If you could give it a test I would really appreciate it.

foal commented 7 years ago

I think no. The referenced changeset is about destination interfaces. In my case convertor is registered for source interface.

foal commented 7 years ago

Another workaround is override canConvert method with implementation from 1.4.6 version

@Override
public boolean canConvert(final Type<?> sourceType, final Type<?> destinationType) {
    return this.sourceType.isAssignableFrom(sourceType) && this.destinationType.equals(destinationType);
}
brabenetz commented 7 years ago

I like the "exact" compare as default implementation. Overwriting the canConvert Method with a "similar" comparison, feels for me the normal way and not like a workaround. But sure, it is a difference between 1.4.6 and 1.5.0. Good that you mentioned it. Other developers have maybe the same problem during update.

foal commented 7 years ago

What was the reason to change convertor behaviour? I feel I do something wrong :) IMHO the old canConvert is more essential.

brabenetz commented 7 years ago

I thinks (~2 years ago, the commit "FIXED Builtin Converters") some built-in converters for Date Time conversions where added. And java.sql.Date to java.util.Date Converter doesn't work with isAssignableFrom because java.sql.Date extends java.util.Date. The same applies for java.sql.Time and java.sql.Timestamp. With isAssignableFrom, in many cases multiple Converters would match the requirements, and only your luck would decide which one will be used.

Btw,: I prefer to use extension of CustomMapper (and maybe ObjectFactory) where possible. At least in my cases I had no much need to extends CustomConverter.

Quist commented 7 years ago

We are experiencing the same issue while upgrading from 1.4.2 to 1.5.1. In our case, we have a converter between XMLGregorianCalendar and LocalDate. However, in runtime, an XMLGregorianCalendarImpl is passed to the mapper which is not accepted by the canConvert function.

To me, it seems that reverting the changes in the canConvert function would solve this issue.