feat(java): Invoke callback on read failure #1870
Conversation
yoohaemin
force-pushed
the
yoohaemin/callback-on-read-failure
branch
from
b1e75ba to
63ac97a
Compare
| // Field doesn't exist in current class, invoke field mismatch callback. | ||
| Expression fieldMismatchCallback = | ||
| Expression.Invoke.inlineInvoke( | ||
| new Expression.Reference(FURY_NAME, TypeRef.of(Fury.class)), | ||
| "getFieldMismatchCallback", | ||
| TypeRef.of(FieldMismatchCallback.class), | ||
| /* needTryCatch */ false); |
There was a problem hiding this comment.
This can be a private field, instead of being instantiated every time. I'm not used to this kind of metaprogramming so I need to figure out how to do this.
| @@ -336,6 +345,11 @@ public FuryBuilder withName(String name) { | |||
| return this; | |||
| } | |||
|
|
|||
| public FuryBuilder withFieldMismatchCallback(FieldMismatchCallback callback) { | |||
| this.fieldMismatchCallback = callback; | |||
There was a problem hiding this comment.
Maybe it can be confusing that the callback can be overridden, not added on top of already registered ones?
| @@ -784,6 +784,135 @@ static boolean readBasicObjectFieldValueFailed( | |||
| } | |||
| } | |||
|
|
|||
| static Object readFieldValue( | |||
There was a problem hiding this comment.
This is a rather big method; I'm not sure if HotSpot can compile this. In any case, this shouldn't be in the hot path.
|
Is it possible to deserialize again by taking fields that don't exist in the class and unifying them in a map property? |
| if (buffer.readByte() == Fury.NULL_FLAG) { | ||
| return null; | ||
| } else if (fury.isBasicTypesRefIgnored()) { | ||
| return readFinalObjectFieldValue( |
There was a problem hiding this comment.
Seems this branch can be removed
| if (buffer.readByte() == Fury.NULL_FLAG) { | ||
| return null; | ||
| } else if (fury.isBasicTypesRefIgnored()) { | ||
| return readFinalObjectFieldValue( |
| if (buffer.readByte() == Fury.NULL_FLAG) { | ||
| return null; | ||
| } else if (fury.isBasicTypesRefIgnored()) { | ||
| return readFinalObjectFieldValue( |
| new Literal(descriptor.getTypeName()), | ||
| new Literal(descriptor.getName())); | ||
|
|
||
| Expression invokeHandler = |
There was a problem hiding this comment.
The onMismatch can be cached as a field using getOrCreateField function
| "forName", | ||
| TypeRef.of(Class.class), | ||
| true, | ||
| new Literal(beanClass.getName())), |
There was a problem hiding this comment.
Use Literal.ofXXX instead
| fieldMismatchCallback, | ||
| "onMismatch", | ||
| TypeRef.of(FieldMismatchCallback.FieldAdjustment.class), | ||
| new Expression.StaticInvoke( |
There was a problem hiding this comment.
Use org.apache.fury.builder.CodecBuilder#beanClassExpr(java.lang.Class<?>) instead
|
Hi @yoohaemin , is there anything I can help to put this PR forward? |
|
@chaokunyang hey, thanks for the ping. I haven't forgotten about this PR, but I am a bit swamped with other things personally and have no time to push it forward. If you want to push it forward yourself, please feel free to do so. Otherwise, I will work on it when I have time. Unfortunately, I can't tell you when exactly I will have time, but probably this month would be hard. |
|
@yoohaemin thanks for continuing working on this pr. This ia not urgent, you can take your time. I will take a look at the ci failures to see whether I can provide more inputs |
What does this PR do?
See discussion: #1848 (comment)
When Fury encounters a value that does not exist in the current class definition, it can still read the value. This PR allows the user of the library to specify what to do with the value. The user can specify a function
Object -> Objectto update the value, and also specify aFieldfor that adjusted value to be set.Related issues
#1848
Does this PR introduce any user-facing change?
Benchmark
TODO