Skip to content

fix AuthorityRuleEntity and ParamFlowRuleEntity fastjson serialize problem#889

Merged
sczyh30 merged 11 commits intoalibaba:masterfrom
linlinisme:20190704-fix-json
Jul 31, 2019
Merged

fix AuthorityRuleEntity and ParamFlowRuleEntity fastjson serialize problem#889
sczyh30 merged 11 commits intoalibaba:masterfrom
linlinisme:20190704-fix-json

Conversation

@linlinisme
Copy link
Copy Markdown
Collaborator

@linlinisme linlinisme commented Jul 4, 2019

Describe what this PR does / why we need it

the ParamFlowRuleEntity and AuthorityRuleEntity json serialize is wrong when they use com.alibaba.fastjson.JSON#toJSONString(java.lang.Object). Because the @JsonIgnore annotation is not belong to fastjson, but com.fasterxml.jackson. They are different json lib. So
com.alibaba.fastjson.JSON#toJSONString(java.lang.Object) still json serialize the field though it modified with @JsonIgnore

Does this pull request fix one issue?

fix #948

Describe how you did it

add fastjson's annotation @JSONField(serialize = false) at the base of @JsonIgnore

Describe how to verify it

run test cases :com.alibaba.csp.sentinel.dashboard.datasource.entity.JsonSerializeTest

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 4, 2019

Codecov Report

Merging #889 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #889   +/-   ##
=========================================
  Coverage     42.25%   42.25%           
- Complexity     1419     1422    +3     
=========================================
  Files           307      307           
  Lines          8894     8894           
  Branches       1203     1203           
=========================================
  Hits           3758     3758           
- Misses         4677     4679    +2     
+ Partials        459      457    -2
Impacted Files Coverage Δ Complexity Δ
...tinel/slots/block/flow/param/ParamFlowChecker.java 52.7% <0%> (-2.71%) 28% <0%> (-1%)
...a/com/alibaba/csp/sentinel/node/StatisticNode.java 56.81% <0%> (+1.13%) 29% <0%> (+2%) ⬆️
...a/csp/sentinel/slots/statistic/base/LeapArray.java 72.27% <0%> (+1.98%) 35% <0%> (+1%) ⬆️
...ava/com/alibaba/csp/sentinel/node/ClusterNode.java 100% <0%> (+4.76%) 8% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c14e329...1524604. Read the comment docs.

@linlinisme linlinisme changed the title fix @JsonIgnore annotation don't work with alibaba fastjson problem fix AuthorityRuleEntity and ParamFlowRuleEntity fastjson serialize problem Jul 4, 2019
@sczyh30 sczyh30 added the area/dashboard Issues or PRs about Sentinel Dashboard label Jul 4, 2019
Copy link
Copy Markdown
Member

@sczyh30 sczyh30 left a comment

Choose a reason for hiding this comment

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

LGTM

@sczyh30 sczyh30 merged commit 88b5631 into alibaba:master Jul 31, 2019
@sczyh30
Copy link
Copy Markdown
Member

sczyh30 commented Jul 31, 2019

Thanks!

For V2 controllers (rules for the entire application) we may need another good solution. Maybe we could just push the origin ParamFlowRule instead of the rule entity (the ip and port is deprecated in V2 scenarios). Maybe the RuleRepository should also be refactored to adapt this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/dashboard Issues or PRs about Sentinel Dashboard

Projects

None yet

3 participants