feat(dart): Introduce id based enum serialization#3482
feat(dart): Introduce id based enum serialization#3482chaokunyang merged 14 commits intoapache:mainfrom
Conversation
analyze if the input is valid and pass to spec update EnumSpec to support idToValue map for id-based enum serialization
30a3919 to
1c56dfe
Compare
|
Firstly, Thank you sm for the clarification about this in the discussions, it took some time to complete it.
|
|
Could we also support this pattern? @ ForyEnum()
enum UserRole {
guest(0),
member(1),
admin(2);
@ForyEnumId()
final int code;
const UserRole(this.code);
} |
|
|
||
| part '../generated/enum_id_foo.g.dart'; | ||
|
|
||
| @foryEnum |
There was a problem hiding this comment.
Yes, we should do it as per conventions, but the existing files also follow the same pattern. If you prefer, shall I make a separate PR for that first before this one?
Yes, I had this in mind initially too. I went with per-value annotation because I feel the id is a metadata for values and since Dart enums don't support private fields, so the |
|
Sometimes users also want to use that id too. So I think we could support both. Could you support both in this pr? |
0aa30d2 to
dbcc164
Compare
|
Yes, we can do that! I have updated the PR to support both methods and added the relevant tests. |
| final List<String> missingIdValues = <String>[]; | ||
|
|
||
| for (final FieldElement enumField in enumFields) { | ||
| final int? id = idFieldName != null |
There was a problem hiding this comment.
Validate @ForyEnumId range before generating enum specs
This accepts any int from @ForyEnumId and later feeds it to writeVarUint32Small7(...). Negative values do not fail cleanly here: I reproduced writeVarUint32Small7(-1) decoding back as 34359738367, so @ForyEnumId(-1) silently writes a different wire id than the one the user declared. Please reject ids outside the unsigned 32-bit range during analysis, and ideally keep a serializer-side assertion as defense in depth.
|
|
||
| void main() { | ||
| group('Enum serializer', () { | ||
| test('writes and reads annotated enum ids when all values are annotated', |
There was a problem hiding this comment.
Add a public Fory round-trip for annotated enums
These tests prove the generated EnumSpec shape and the direct EnumSerializer behavior, but they still do not exercise the public Fory.serialize / Fory.deserialize path with an annotated enum. That leaves a gap around the real registration and resolver wiring for this new feature. Please add at least one end-to-end round-trip test for an enum that uses @ForyEnumId.
|
Thanks for the review! Now, For the codegen path, if Ids are out of range it fall back to ordinal serialization with a warning while |
Need strict errors instead |
f098b12 to
a4f5976
Compare
|
I've added the round-trip tests and updated for out-of-range. Should |
Yes. missingIdValues and duplicateIds should also be strict. The rule should be:
|
|
Thanks! I have made the revision. |
Why?
The Dart module, only supports ordinal enum serialization, where each enum
value is serialized as its position index (0, 1, 2...), which breaks in different
cases like adding or reordering values. This adds optional id based serialization
with so that we can provide unique id to every while preserving ordinal serialization
as the default fallback.
What does this PR do?
- partial @ForyEnumId usage
- duplicate IDs
Related issues
AI Contribution Checklist
AI Usage Disclosure
AI assistance was used to suggest tests and for better warnings in enum_analyzer.
dart/packages/fory/lib/src/codegen/analyze/impl/struct/enum_analyzer_impl.dart
dart/packages/fory-test/test/
Does this PR introduce any user-facing change?
@ForyEnumIdis a new annotation users will write in their codeForyEnumIdclass is publicly accessibleBenchmark
General struct performance remained similar because those benchmarks do not exercise enum serialization paths.