-
Notifications
You must be signed in to change notification settings - Fork 262
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
feat(java): deserialize one pojo into another type #2012
Conversation
@@ -22,7 +22,7 @@ | |||
import org.apache.fury.type.TypeUtils; | |||
|
|||
public class ClassSpec { | |||
public final String entireClassName; | |||
public String entireClassName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to keep ClassDef immutable abd return a new ClassDef instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure ofc, im still stuck facing exception here and there.
128b203
to
04a9e14
Compare
Hi @chaokunyang, need your help here. |
ClassInfo classInfo = readClassInfoWithMetaShare(buffer, fury.getSerializationContext().getMetaContext()); | ||
|
||
// replace target class if needed | ||
if (!targetClass.getName().equals(classInfo.getCls().getName())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!targetClass.getName().equals(classInfo.getCls().getName())) { | |
if (targetClass != classInfo.getCls()) { |
this will be much faster
ClassInfo targetClassInfo = getClassInfo(targetClass); | ||
buildClassDef(targetClassInfo); | ||
classInfo.classDef = ClassDef.replaceRootClassTo(this, targetClassInfo); | ||
classInfo.serializer = createSerializer(targetClass); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line won't work as you expect. It just create a serializer for raw targetClass
without using type info from peer class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I'll look into that first
@orisgarno I make it work in 52dfea2 , you can continue the work now. And it seems the tests you write have some issues, please take a look at it. |
Hi @orisgarno , could you also update the user guide document about how to deserialize one type into another? |
@chaokunyang this pr making it auto for the root class right? Anything specific you want to add? |
I think https://github.com/apache/fury/blob/main/docs/guide/java_serialization_guide.md#copingmapping-object-from-one-type-to-another-type needs being updated. Class registration for deserialization is unnecessary |
my bad, i have removed the unnecessary part. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
<!-- **Thanks for contributing to Fury.** **If this is your first time opening a PR on fury, you can refer to [CONTRIBUTING.md](https://github.com/apache/fury/blob/main/CONTRIBUTING.md).** Contribution Checklist - The **Apache Fury (incubating)** community has restrictions on the naming of pr titles. You can also find instructions in [CONTRIBUTING.md](https://github.com/apache/fury/blob/main/CONTRIBUTING.md). - Fury has a strong focus on performance. If the PR you submit will have an impact on performance, please benchmark it first and provide the benchmark result here. --> replace class def if target class is different type with the actual serialized one, so it can be deserialized to another type <!-- Describe the purpose of this PR. --> <!-- Is there any related issue? Please attach here. - #xxxx0 - #xxxx1 - #xxxx2 --> <!-- If any user-facing interface changes, please [open an issue](https://github.com/apache/fury/issues/new/choose) describing the need to do so and update the document if necessary. --> - [ ] Does this PR introduce any public API change? - [ ] Does this PR introduce any binary protocol compatibility change? <!-- When the PR has an impact on performance (if you don't know whether the PR will have an impact on performance, you can submit the PR first, and if it will have impact on performance, the code reviewer will explain it), be sure to attach a benchmark data here. --> --------- Co-authored-by: chaokunyang <[email protected]>
What does this PR do?
replace class def if target class is different type with the actual serialized one, so it can be deserialized to another type
#1998
Related issues
Does this PR introduce any user-facing change?
Benchmark