feat(java): ThreadSafeFury add getClassResolver method #1780
Conversation
| ThreadLocalFury threadLocalFury = | ||
| builder().withCodegen(false).withRefCopy(true).buildThreadLocalFury(); | ||
| ThreadLocalFury threadLocalFury = builder().withCodegen(false).withRefCopy(true).buildThreadLocalFury(); | ||
| threadLocalFury.getClassResolver().setClassChecker((classResolver, className1) -> true); |
There was a problem hiding this comment.
This won't set set class checker for other fury instance. We could set Class checker when building threadsafefury by creating threadlocalfury directly
There was a problem hiding this comment.
This won't set set class checker for other fury instance. We could set Class checker when building threadsafefury by creating threadlocalfury directly
I believe that in some cases, adding check listeners and serializers involves dynamically adding or modifying them at runtime, which cannot be predicted with absolute certainty
There was a problem hiding this comment.
There was a problem hiding this comment.
If so, maybe we need to add setClassChecker to BaseFury/ThreadSafeFur directly.
There was a problem hiding this comment.
If so, maybe we need to add
setClassCheckerto BaseFury/ThreadSafeFur directly.
That's a good idea. I will make modifications based on this suggestion
|
Hi @funky-eyes , the rust ci failure is not related to your changes, I will fix this soon, track the ci failure issue at #1781 |
thx |
| public ClassResolver getClassResolver() { | ||
| return bindingThreadLocal.get().get().getClassResolver(); | ||
| public void setClassChecker(ClassChecker classChecker) { | ||
| bindingThreadLocal.get().get().getClassResolver().setClassChecker(classChecker); |
There was a problem hiding this comment.
We need to use processCallback
There was a problem hiding this comment.
We need to use processCallback
I don't quite understand what you mean. Could you give me an example?
There was a problem hiding this comment.
You can take setSerializerFactory as an example:
public void setSerializerFactory(SerializerFactory serializerFactory) {
processCallback(fury -> fury.setSerializerFactory(serializerFactory));
}There was a problem hiding this comment.
And maybe we can add following API for extending:
public void configureClassResolver(Consumer<ClassResolver> apply);There was a problem hiding this comment.
You can take
setSerializerFactoryas an example:public void setSerializerFactory(SerializerFactory serializerFactory) { processCallback(fury -> fury.setSerializerFactory(serializerFactory)); }
I feel doing it this way might be simpler.
| } | ||
|
|
||
| @Override | ||
| public SerializerFactory getSerializerFactory() { |
There was a problem hiding this comment.
We should not provide this api. It may return different object when classloader changes or we invoke It from another thread
There was a problem hiding this comment.
You are correct; I will proceed with the corrections
| @@ -265,6 +267,16 @@ public ClassLoader getClassLoader() { | |||
| return bindingThreadLocal.get().getClassLoader(); | |||
| } | |||
|
|
|||
| @Override | |||
| public ClassChecker getClassChecker() { | |||
There was a problem hiding this comment.
Please implement this in AbstractThreadSafeFury
There was a problem hiding this comment.
Please implement this in AbstractThreadSafeFury
AbstractThreadSafeFury does not have bindingThreadLocal, so it seems unable to implement this interface.
There was a problem hiding this comment.
You can invoke processCallback instead
There was a problem hiding this comment.
You can invoke processCallback instead
processCallback ReturnType is void
There was a problem hiding this comment.
I think you can create ClassChecker first and set it to Fury. Then you don't need to add any API to Fury. We won't add such API to get Classchecker or ClassResolver/SerializerFactory
There was a problem hiding this comment.
We can only allow set ClassChecker
There was a problem hiding this comment.
We can only allow set ClassChecker
ok
What does this PR do?
Because using ThreadSafeFury cannot get ClassResolver, so I submitted this pr to increase some checklistener and other behavior
Related issues
Does this PR introduce any user-facing change?
Benchmark