Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
qixiaobo opened this issue Sep 27, 2021 · 3 comments

Comments

@qixiaobo
Copy link

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
Copy link
Author

Especially orika used to use this cache

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

@qixiaobo
Copy link
Author

image

@qixiaobo
Copy link
Author

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant