orika-mapper/orika

Orika cannot find correct converter with class type define equals to compare typekey

qixiaobo opened this issue · 3 comments

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.

Especially orika used to use this cache

    private final ConcurrentLinkedHashMap<Key, MappingStrategy> strategyCache = getConcurrentLinkedHashMap(500);