Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix#1254#1305 bug on JSON serialization and deserialization #1306

Merged
merged 10 commits into from
Feb 28, 2023

Conversation

gofow
Copy link
Contributor

@gofow gofow commented Feb 19, 2023

Motivation:

Fix#1254#1305 bug on JSON serialization and deserialization

Modification:

  1. In BeanSerializer.serialize(), create a fresh Map when the "bean" is a Map instance, in order to prevent any unpredictable changes due to references
  2. In BeanSerializer.serialize(), use class.getName() rather than class.getCanonicalName() when the bean is a custom object, in order to get correct representation of inner class for class loader
  3. Add two cases to JSONTest

@sofastack-bot
Copy link

sofastack-bot bot commented Feb 19, 2023

Hi @gofow, welcome to SOFAStack community, Please sign Contributor License Agreement!

After you signed CLA, we will automatically sync the status of this pull request in 3 minutes.

@sofastack-bot sofastack-bot bot added bug Something isn't working cla:no Need sign CLA First-time contributor First-time contributor size/M labels Feb 19, 2023
@EvenLjj
Copy link
Collaborator

EvenLjj commented Feb 20, 2023

image
@gofow Pls format code by mvn clean compile before git push.

@sofastack-bot sofastack-bot bot added cla:yes CLA is ok and removed cla:no Need sign CLA labels Feb 20, 2023
@gofow
Copy link
Contributor Author

gofow commented Feb 20, 2023

@EvenLjj Thank you! Actually I'm feeling confused about the error, and I have done with the format.

Copy link
Collaborator

@EvenLjj EvenLjj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codecov
Copy link

codecov bot commented Feb 27, 2023

Codecov Report

Merging #1306 (19fdf5a) into master (75bc1ff) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #1306      +/-   ##
============================================
- Coverage     71.97%   71.96%   -0.02%     
  Complexity      783      783              
============================================
  Files           415      415              
  Lines         17651    17651              
  Branches       2753     2753              
============================================
- Hits          12705    12703       -2     
- Misses         3538     3546       +8     
+ Partials       1408     1402       -6     
Impacted Files Coverage Δ
...om/alipay/sofa/rpc/common/json/BeanSerializer.java 87.07% <100.00%> (+0.56%) ⬆️
...java/com/alipay/sofa/rpc/module/LookoutModule.java 52.38% <0.00%> (-33.34%) ⬇️
...n/java/com/alipay/sofa/rpc/common/SofaConfigs.java 84.90% <0.00%> (-1.89%) ⬇️
...ay/sofa/rpc/client/AllConnectConnectionHolder.java 60.76% <0.00%> (-0.26%) ⬇️
...lipay/sofa/rpc/message/AbstractResponseFuture.java 57.01% <0.00%> (+0.87%) ⬆️
.../com/alipay/sofa/rpc/context/RpcInvokeContext.java 82.40% <0.00%> (+0.92%) ⬆️
...com/alipay/sofa/rpc/context/RpcRuntimeContext.java 90.12% <0.00%> (+1.23%) ⬆️
...pay/sofa/rpc/transport/ClientTransportFactory.java 78.46% <0.00%> (+1.53%) ⬆️
...n/java/com/alipay/sofa/rpc/log/TimeWaitLogger.java 84.00% <0.00%> (+4.00%) ⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@EvenLjj EvenLjj added this to the 5.9.2 milestone Feb 27, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working cla:yes CLA is ok First-time contributor First-time contributor size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants