chaokunyang commented on code in PR #3021:
URL: https://github.com/apache/fory/pull/3021#discussion_r2625961552
##########
java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java:
##########
@@ -432,10 +457,10 @@ public static SortedMap<Member, Descriptor>
getAllDescriptorsMap(Class<?> clz) {
});
public static SortedMap<Member, Descriptor> getAllDescriptorsMap(
- Class<?> clz, boolean searchParent) {
+ Class<?> clz, boolean searchParent, boolean globalRefTrackingEnabled) {
try {
Tuple2<SortedMap<Member, Descriptor>, SortedMap<Member, Descriptor>>
tuple2 =
- descCache.get(clz, () -> createAllDescriptorsMap(clz));
+ descCache.get(clz, () -> createAllDescriptorsMap(clz,
globalRefTrackingEnabled));
Review Comment:
This is flaky, if `createAllDescriptorsMap` is invoked with
`globalRefTrackingEnabled = true`, but later invoked with
`globalRefTrackingEnabled=false`, then later invoke won't take efflect.
I prefer not passing parameters to
`getAllDescriptorsMap/createAllDescriptorsMap`. We build Descriptor with
default field annotation and whether it's primitive.
But when we get Descriptors from `TypeResolver/ClassResolver/XtypeResolver`,
we create new `Descriptor` with correct settings. When we create new
Descriptors, we should recompute based on `ForyField` annotation and `ref
tracking` flag.
Then in all serializers, we can take `trackingRef/nullable` are real truth.
##########
java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java:
##########
@@ -432,10 +457,10 @@ public static SortedMap<Member, Descriptor>
getAllDescriptorsMap(Class<?> clz) {
});
public static SortedMap<Member, Descriptor> getAllDescriptorsMap(
- Class<?> clz, boolean searchParent) {
+ Class<?> clz, boolean searchParent, boolean globalRefTrackingEnabled) {
try {
Tuple2<SortedMap<Member, Descriptor>, SortedMap<Member, Descriptor>>
tuple2 =
- descCache.get(clz, () -> createAllDescriptorsMap(clz));
+ descCache.get(clz, () -> createAllDescriptorsMap(clz,
globalRefTrackingEnabled));
Review Comment:
This is flaky, if `createAllDescriptorsMap` is invoked with
`globalRefTrackingEnabled = true`, but later invoked with
`globalRefTrackingEnabled=false`, then later invoke won't take efflect.
I prefer not passing parameters to
`getAllDescriptorsMap/createAllDescriptorsMap`. We build Descriptor with
default field annotation and whether it's primitive.
But when we get Descriptors from `TypeResolver/ClassResolver/XtypeResolver`,
we create new `Descriptor` with correct settings. When we create new
Descriptors, we should recompute based on `ForyField` annotation and `ref
tracking` flag.
Then in all serializers, we can take `trackingRef/nullable` as real truth.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
