feat(java): Add EnumNameSerializer for Enum #1791
Conversation
java/fury-core/src/main/java/org/apache/fury/config/FuryBuilder.java
Outdated
Show resolved
Hide resolved
java/fury-core/src/main/java/org/apache/fury/config/Config.java
Outdated
Show resolved
Hide resolved
java/fury-core/src/main/java/org/apache/fury/resolver/EnumStringResolver.java
Outdated
Show resolved
Hide resolved
java/fury-core/src/main/java/org/apache/fury/serializer/EnumNameSerializer.java
Outdated
Show resolved
Hide resolved
java/fury-core/src/main/java/org/apache/fury/serializer/EnumNameSerializer.java
Outdated
Show resolved
Hide resolved
|
Hi @xxd819240366 , thanks for contribuing to Apache Fury. The code looks to me overall, I left some minor comments. And would you like to update |
|
Ok，I'll update it again |
…e/fury into 0.7.1_add_enum_name_serializer
|
By the way, I can pass all test on my local development environment with java 8，but can't pass test on Java CI(8) . There is a error in org.apache.fury.serializer.EnumSerializerTest.testEnumSubclassFieldCompatible(org.apache.fury.serializer.EnumSerializerTest) only happen on github CI pipline |
|
Seems some your jar version is 0.7.0 but others are 0.8.0-SNAPSHOT. Could you update all mavem module to 0.8.0-SNAPSHOT? |
|
@chaokunyang Seems all maven module version already updated to 0.8.0-SNAPSHOT |
| public Enum read(MemoryBuffer buffer) { | ||
| MetaStringBytes metaStringBytes = metaStringResolver.readMetaStringBytes(buffer); | ||
| if (!enumMap.containsKey(metaStringBytes)) { | ||
| Optional<NonexistentEnum> first = enumMap.values().stream().findFirst(); |
There was a problem hiding this comment.
if deserializeNonexistentEnumValueAsNull is enabled, we should return null here, otherwise we should throw Exception
There was a problem hiding this comment.
Or instead of returning null, we could return NonexistentEnum.UNKNOWN instead
| Optional<NonexistentEnum> first = enumMap.values().stream().findFirst(); | ||
| return first.orElse(null); | ||
| } | ||
| return enumMap.get(metaStringBytes); |
There was a problem hiding this comment.
This introduce a second map loopkup, we should avoid it. we can value = enumMap.get(xxx); and check whether value is null instead of check by containsKey
|
|
|
@lionel-xie Could you merge fury main branch first, your base branch is 0.7.0, not the developer main branch |
|
done in #1892 |
What does this PR do?
Add EnumNameSerializer for Enum, can config it by org.apache.fury.config.Config#useEnumNameSerializer, default value is false
Related issues
Does this PR introduce any user-facing change?
Benchmark