Skip to content

Conversation

@fllife
Copy link

@fllife fllife commented Aug 12, 2020

Describe what this PR does / why we need it

in ModifyRulesCommandHandler linenumber 66:

List<FlowRule> flowRules = JSONArray.parseArray(data, FlowRule.class);
FlowRuleManager.loadRules(flowRules);

when I use fastjson 1.2.7, field resourse and` limitApp can not be set value ,cause config rule not take effect because class AbstractRule do not have setter method

Does this pull request fix one issue?

fixd #1661

Describe how you did it

class AbstractRulemake setter method and not return value

Describe how to verify it

when I use fastjson 1.2.7, filed resourse and limitApp can be set value and config rule can take effect

Special notes for reviews

fllife added 2 commits August 12, 2020 14:43
fixd alibaba#1661: fastjson deserialization problem
@CLAassistant
Copy link

CLAassistant commented Aug 12, 2020

CLA assistant check
All committers have signed the CLA.

Copy link
Author

@fllife fllife left a comment

Choose a reason for hiding this comment

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

fixed #1661: fastjson deserialize require a setter method

@jasonjoo2010
Copy link
Collaborator

Actually fastjson before 1.2.12 (exclusive) is not usable because it's buggy to those classes having inheritance.
Refer to https://github.com/alibaba/Sentinel/blob/master/sentinel-transport/sentinel-transport-common/src/main/java/com/alibaba/csp/sentinel/command/handler/ModifyRulesCommandHandler.java#L54

And another solution is under progress on these kinds of mess on #1739
You can have a look or do some reviews kindly.

@sczyh30
Copy link
Member

sczyh30 commented Nov 10, 2020

Closing this PR due to no further feedback. Thanks for contributing.

@sczyh30 sczyh30 closed this Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants