Orika cannot find correct converter with class type define equals to compare typekey
qixiaobo opened this issue · 3 comments
qixiaobo commented
Orika converter may works like below
public boolean canConvert(Type<?> sourceType, Type<?> destinationType) {
return this.sourceType.equals(sourceType) && destinationType.equals(this.destinationType);
}
So we need equals to support
But we find this
@Override
public boolean equals(final Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (getClass() != obj.getClass()) {
return false;
}
Type<?> other = (Type<?>) obj;
return this.key.equals(other.key);
}
We need check typeKey again ??? Even class is same???
We go on with TypeKey
public boolean equals(Object other) {
if (this == other)
return true;
if (other == null)
return false;
if (other.getClass() != getClass())
return false;
TypeKey otherKey = (TypeKey) other;
return Arrays.equals(this.bytes, otherKey.bytes);
}
So we must know the bytes how to computed
static TypeKey valueOf(Class<?> rawType, java.lang.reflect.Type[] typeArguments) {
byte[] identityHashBytes = new byte[(typeArguments.length + 1) * 4];
intToByteArray(getTypeIndex(rawType), identityHashBytes, 0);
for (int i = 0, len = typeArguments.length; i < len; ++i) {
intToByteArray(getTypeIndex(typeArguments[i]), identityHashBytes, i + 1);
}
return new TypeKey(identityHashBytes);
}
Ok we can see getTypeIndex
private static int getTypeIndex(java.lang.reflect.Type type) {
Integer typeIndex = knownTypes.get(type);
if (typeIndex == null) {
synchronized (type) {
typeIndex = knownTypes.get(type);
if (typeIndex == null) {
typeIndex = currentIndex.getAndAdd(1);
knownTypes.put(type, typeIndex);
}
}
}
return typeIndex;
}
Sadly
private static volatile WeakHashMap<java.lang.reflect.Type, Integer> knownTypes = new WeakHashMap<java.lang.reflect.Type, Integer>();
This is not thread safe.
So it may cause key missing in high concurrency.
Even in 1.5.4 Orika change to
private static volatile Map<java.lang.reflect.Type, Integer> knownTypes = Collections.synchronizedMap(new WeakHashMap<java.lang.reflect.Type, Integer>());
It may reduce this problem.
But why we need check this typekey???
You should know this converter may be registered as early as possible.
So if the type changes then converter may not work as expected.
qixiaobo commented
Especially orika used to use this cache
private final ConcurrentLinkedHashMap<Key, MappingStrategy> strategyCache = getConcurrentLinkedHashMap(500);
qixiaobo commented
qixiaobo commented